diff --git a/spring-session-core/src/main/java/org/springframework/session/SaveMode.java b/spring-session-core/src/main/java/org/springframework/session/SaveMode.java new file mode 100644 index 00000000..a3a2a3c4 --- /dev/null +++ b/spring-session-core/src/main/java/org/springframework/session/SaveMode.java @@ -0,0 +1,49 @@ +/* + * Copyright 2014-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.session; + +/** + * Supported modes of tracking and saving session changes to session store. + * + * @author Rob Winch + * @author Vedran Pavic + * @since 2.2.0 + */ +public enum SaveMode { + + /** + * Save only changes made to session, for instance using + * {@link Session#setAttribute(String, Object)}. In highly concurrent environments, + * this mode minimizes the risk of attributes being overwritten during processing of + * parallel requests. + */ + ON_SET_ATTRIBUTE, + + /** + * Same as {@link #ON_SET_ATTRIBUTE} with addition of saving attributes that have been + * read using {@link Session#getAttribute(String)}. + */ + ON_GET_ATTRIBUTE, + + /** + * Always save all session attributes, regardless of the interaction with the session. + * In highly concurrent environments, this mode increases the risk of attributes being + * overwritten during processing of parallel requests. + */ + ALWAYS + +} diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java index f59c4d82..03e78513 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java @@ -28,6 +28,7 @@ import reactor.core.publisher.Mono; import org.springframework.data.redis.core.ReactiveRedisOperations; import org.springframework.session.MapSession; import org.springframework.session.ReactiveSessionRepository; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.util.Assert; @@ -59,6 +60,8 @@ public class ReactiveRedisOperationsSessionRepository */ private Integer defaultMaxInactiveInterval; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + public ReactiveRedisOperationsSessionRepository(ReactiveRedisOperations sessionRedisOperations) { Assert.notNull(sessionRedisOperations, "sessionRedisOperations cannot be null"); this.sessionRedisOperations = sessionRedisOperations; @@ -90,6 +93,15 @@ public class ReactiveRedisOperationsSessionRepository Assert.notNull(redisFlushMode, "redisFlushMode cannot be null"); } + /** + * Set the save mode. + * @param saveMode the save mode + */ + public void setSaveMode(SaveMode saveMode) { + Assert.notNull(saveMode, "saveMode must not be null"); + this.saveMode = saveMode; + } + /** * Returns the {@link ReactiveRedisOperations} used for sessions. * @return the {@link ReactiveRedisOperations} used for sessions @@ -102,12 +114,11 @@ public class ReactiveRedisOperationsSessionRepository @Override public Mono createSession() { return Mono.defer(() -> { - RedisSession session = new RedisSession(); - + MapSession cached = new MapSession(); if (this.defaultMaxInactiveInterval != null) { - session.setMaxInactiveInterval(Duration.ofSeconds(this.defaultMaxInactiveInterval)); + cached.setMaxInactiveInterval(Duration.ofSeconds(this.defaultMaxInactiveInterval)); } - + RedisSession session = new RedisSession(cached, true); return Mono.just(session); }); } @@ -132,7 +143,7 @@ public class ReactiveRedisOperationsSessionRepository .filter((map) -> !map.isEmpty()) .map(new RedisSessionMapper(id)) .filter((session) -> !session.isExpired()) - .map(RedisSession::new) + .map((session) -> new RedisSession(session, false)) .switchIfEmpty(Mono.defer(() -> deleteById(id).then(Mono.empty()))); // @formatter:on } @@ -168,27 +179,20 @@ public class ReactiveRedisOperationsSessionRepository private String originalSessionId; - /** - * Creates a new instance ensuring to mark all of the new attributes to be - * persisted in the next save operation. - */ - RedisSession() { - this(new MapSession()); - this.delta.put(RedisSessionMapper.CREATION_TIME_KEY, getCreationTime().toEpochMilli()); - this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) getMaxInactiveInterval().getSeconds()); - this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, getLastAccessedTime().toEpochMilli()); - this.isNew = true; - } - - /** - * Creates a new instance from the provided {@link MapSession}. - * @param mapSession the {@link MapSession} that represents the persisted session - * that was retrieved. Cannot be null. - */ - RedisSession(MapSession mapSession) { - Assert.notNull(mapSession, "mapSession cannot be null"); - this.cached = mapSession; - this.originalSessionId = mapSession.getId(); + RedisSession(MapSession cached, boolean isNew) { + this.cached = cached; + this.isNew = isNew; + this.originalSessionId = cached.getId(); + if (this.isNew) { + this.delta.put(RedisSessionMapper.CREATION_TIME_KEY, cached.getCreationTime().toEpochMilli()); + this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, + (int) cached.getMaxInactiveInterval().getSeconds()); + this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, cached.getLastAccessedTime().toEpochMilli()); + } + if (this.isNew || (ReactiveRedisOperationsSessionRepository.this.saveMode == SaveMode.ALWAYS)) { + getAttributeNames().forEach((attributeName) -> this.delta.put(getAttributeKey(attributeName), + cached.getAttribute(attributeName))); + } } @Override @@ -203,7 +207,12 @@ public class ReactiveRedisOperationsSessionRepository @Override public T getAttribute(String attributeName) { - return this.cached.getAttribute(attributeName); + T attributeValue = this.cached.getAttribute(attributeName); + if (attributeValue != null + && ReactiveRedisOperationsSessionRepository.this.saveMode.equals(SaveMode.ON_GET_ATTRIBUTE)) { + this.delta.put(getAttributeKey(attributeName), attributeValue); + } + return attributeValue; } @Override diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java index f9f4e0cc..b45dba0e 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java @@ -42,6 +42,7 @@ import org.springframework.session.FlushMode; import org.springframework.session.IndexResolver; import org.springframework.session.MapSession; import org.springframework.session.PrincipalNameIndexResolver; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.session.events.SessionCreatedEvent; import org.springframework.session.events.SessionDeletedEvent; @@ -296,6 +297,8 @@ public class RedisOperationsSessionRepository private FlushMode flushMode = FlushMode.ON_SAVE; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + /** * Creates a new instance. For an example, refer to the class level javadoc. * @param sessionRedisOperations the {@link RedisOperations} to use for managing the @@ -363,6 +366,15 @@ public class RedisOperationsSessionRepository this.flushMode = flushMode; } + /** + * Set the save mode. + * @param saveMode the save mode + */ + public void setSaveMode(SaveMode saveMode) { + Assert.notNull(saveMode, "saveMode must not be null"); + this.saveMode = saveMode; + } + /** * Sets the database index to use. Defaults to {@link #DEFAULT_DATABASE}. * @param database the database index to use @@ -439,7 +451,7 @@ public class RedisOperationsSessionRepository if (!allowExpired && loaded.isExpired()) { return null; } - RedisSession result = new RedisSession(loaded); + RedisSession result = new RedisSession(loaded, false); result.originalLastAccessTime = loaded.getLastAccessedTime(); return result; } @@ -483,9 +495,11 @@ public class RedisOperationsSessionRepository @Override public RedisSession createSession() { - Duration maxInactiveInterval = Duration.ofSeconds((this.defaultMaxInactiveInterval != null) - ? this.defaultMaxInactiveInterval : MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS); - RedisSession session = new RedisSession(maxInactiveInterval); + MapSession cached = new MapSession(); + if (this.defaultMaxInactiveInterval != null) { + cached.setMaxInactiveInterval(Duration.ofSeconds(this.defaultMaxInactiveInterval)); + } + RedisSession session = new RedisSession(cached, true); session.flushImmediateIfNecessary(); return session; } @@ -674,32 +688,22 @@ public class RedisOperationsSessionRepository private String originalSessionId; - /** - * Creates a new instance ensuring to mark all of the new attributes to be - * persisted in the next save operation. - * @param maxInactiveInterval the amount of time that the {@link Session} should - * be kept alive between client requests. - */ - RedisSession(Duration maxInactiveInterval) { - this(new MapSession()); - this.cached.setMaxInactiveInterval(maxInactiveInterval); - this.delta.put(RedisSessionMapper.CREATION_TIME_KEY, getCreationTime().toEpochMilli()); - this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) getMaxInactiveInterval().getSeconds()); - this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, getLastAccessedTime().toEpochMilli()); - this.isNew = true; - } - - /** - * Creates a new instance from the provided {@link MapSession}. - * @param cached the {@link MapSession} that represents the persisted session that - * was retrieved. Cannot be null. - */ - RedisSession(MapSession cached) { - Assert.notNull(cached, "MapSession cannot be null"); + RedisSession(MapSession cached, boolean isNew) { this.cached = cached; + this.isNew = isNew; this.originalSessionId = cached.getId(); Map indexes = RedisOperationsSessionRepository.this.indexResolver.resolveIndexesFor(this); this.originalPrincipalName = indexes.get(PRINCIPAL_NAME_INDEX_NAME); + if (this.isNew) { + this.delta.put(RedisSessionMapper.CREATION_TIME_KEY, cached.getCreationTime().toEpochMilli()); + this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, + (int) cached.getMaxInactiveInterval().getSeconds()); + this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, cached.getLastAccessedTime().toEpochMilli()); + } + if (this.isNew || (RedisOperationsSessionRepository.this.saveMode == SaveMode.ALWAYS)) { + getAttributeNames().forEach((attributeName) -> this.delta.put(getSessionAttrNameKey(attributeName), + cached.getAttribute(attributeName))); + } } public void setNew(boolean isNew) { @@ -709,7 +713,8 @@ public class RedisOperationsSessionRepository @Override public void setLastAccessedTime(Instant lastAccessedTime) { this.cached.setLastAccessedTime(lastAccessedTime); - this.putAndFlush(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, getLastAccessedTime().toEpochMilli()); + this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, getLastAccessedTime().toEpochMilli()); + flushImmediateIfNecessary(); } @Override @@ -744,7 +749,8 @@ public class RedisOperationsSessionRepository @Override public void setMaxInactiveInterval(Duration interval) { this.cached.setMaxInactiveInterval(interval); - this.putAndFlush(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) getMaxInactiveInterval().getSeconds()); + this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) getMaxInactiveInterval().getSeconds()); + flushImmediateIfNecessary(); } @Override @@ -754,7 +760,12 @@ public class RedisOperationsSessionRepository @Override public T getAttribute(String attributeName) { - return this.cached.getAttribute(attributeName); + T attributeValue = this.cached.getAttribute(attributeName); + if (attributeValue != null + && RedisOperationsSessionRepository.this.saveMode.equals(SaveMode.ON_GET_ATTRIBUTE)) { + this.delta.put(getSessionAttrNameKey(attributeName), attributeValue); + } + return attributeValue; } @Override @@ -765,13 +776,15 @@ public class RedisOperationsSessionRepository @Override public void setAttribute(String attributeName, Object attributeValue) { this.cached.setAttribute(attributeName, attributeValue); - this.putAndFlush(getSessionAttrNameKey(attributeName), attributeValue); + this.delta.put(getSessionAttrNameKey(attributeName), attributeValue); + flushImmediateIfNecessary(); } @Override public void removeAttribute(String attributeName) { this.cached.removeAttribute(attributeName); - this.putAndFlush(getSessionAttrNameKey(attributeName), null); + this.delta.put(getSessionAttrNameKey(attributeName), null); + flushImmediateIfNecessary(); } private void flushImmediateIfNecessary() { @@ -780,11 +793,6 @@ public class RedisOperationsSessionRepository } } - private void putAndFlush(String a, Object v) { - this.delta.put(a, v); - this.flushImmediateIfNecessary(); - } - private void save() { saveChangeSessionId(); saveDelta(); diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepository.java index 2791f3f2..6a4d17b7 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepository.java @@ -26,6 +26,7 @@ import java.util.Set; import org.springframework.data.redis.core.RedisOperations; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.session.SessionRepository; import org.springframework.util.Assert; @@ -52,6 +53,8 @@ public class SimpleRedisOperationsSessionRepository private FlushMode flushMode = FlushMode.ON_SAVE; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + /** * Create a new {@link SimpleRedisOperationsSessionRepository} instance. * @param sessionRedisOperations the {@link RedisOperations} to use for managing @@ -89,9 +92,20 @@ public class SimpleRedisOperationsSessionRepository this.flushMode = flushMode; } + /** + * Set the save mode. + * @param saveMode the save mode + */ + public void setSaveMode(SaveMode saveMode) { + Assert.notNull(saveMode, "saveMode must not be null"); + this.saveMode = saveMode; + } + @Override public RedisSession createSession() { - RedisSession session = new RedisSession(this.defaultMaxInactiveInterval); + MapSession cached = new MapSession(); + cached.setMaxInactiveInterval(this.defaultMaxInactiveInterval); + RedisSession session = new RedisSession(cached, true); session.flushIfRequired(); return session; } @@ -120,7 +134,7 @@ public class SimpleRedisOperationsSessionRepository deleteById(sessionId); return null; } - return new RedisSession(session); + return new RedisSession(session, false); } @Override @@ -141,6 +155,10 @@ public class SimpleRedisOperationsSessionRepository return this.keyNamespace + "sessions:" + sessionId; } + private static String getAttributeKey(String attributeName) { + return RedisSessionMapper.ATTRIBUTE_PREFIX + attributeName; + } + /** * An internal {@link Session} implementation used by this {@link SessionRepository}. */ @@ -154,18 +172,20 @@ public class SimpleRedisOperationsSessionRepository private String originalSessionId; - RedisSession(Duration maxInactiveInterval) { - this(new MapSession()); - this.cached.setMaxInactiveInterval(maxInactiveInterval); - this.delta.put(RedisSessionMapper.CREATION_TIME_KEY, getCreationTime().toEpochMilli()); - this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) getMaxInactiveInterval().getSeconds()); - this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, getLastAccessedTime().toEpochMilli()); - this.isNew = true; - } - - RedisSession(MapSession cached) { + RedisSession(MapSession cached, boolean isNew) { this.cached = cached; + this.isNew = isNew; this.originalSessionId = cached.getId(); + if (this.isNew) { + this.delta.put(RedisSessionMapper.CREATION_TIME_KEY, cached.getCreationTime().toEpochMilli()); + this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, + (int) cached.getMaxInactiveInterval().getSeconds()); + this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, cached.getLastAccessedTime().toEpochMilli()); + } + if (this.isNew || (SimpleRedisOperationsSessionRepository.this.saveMode == SaveMode.ALWAYS)) { + getAttributeNames().forEach((attributeName) -> this.delta.put(getAttributeKey(attributeName), + cached.getAttribute(attributeName))); + } } @Override @@ -180,7 +200,12 @@ public class SimpleRedisOperationsSessionRepository @Override public T getAttribute(String attributeName) { - return this.cached.getAttribute(attributeName); + T attributeValue = this.cached.getAttribute(attributeName); + if (attributeValue != null + && SimpleRedisOperationsSessionRepository.this.saveMode.equals(SaveMode.ON_GET_ATTRIBUTE)) { + this.delta.put(getAttributeKey(attributeName), attributeValue); + } + return attributeValue; } @Override @@ -191,7 +216,8 @@ public class SimpleRedisOperationsSessionRepository @Override public void setAttribute(String attributeName, Object attributeValue) { this.cached.setAttribute(attributeName, attributeValue); - putAttribute(RedisSessionMapper.ATTRIBUTE_PREFIX + attributeName, attributeValue); + this.delta.put(getAttributeKey(attributeName), attributeValue); + flushIfRequired(); } @Override @@ -207,7 +233,8 @@ public class SimpleRedisOperationsSessionRepository @Override public void setLastAccessedTime(Instant lastAccessedTime) { this.cached.setLastAccessedTime(lastAccessedTime); - putAttribute(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, getLastAccessedTime().toEpochMilli()); + this.delta.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, getLastAccessedTime().toEpochMilli()); + flushIfRequired(); } @Override @@ -218,7 +245,8 @@ public class SimpleRedisOperationsSessionRepository @Override public void setMaxInactiveInterval(Duration interval) { this.cached.setMaxInactiveInterval(interval); - putAttribute(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) getMaxInactiveInterval().getSeconds()); + this.delta.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) getMaxInactiveInterval().getSeconds()); + flushIfRequired(); } @Override @@ -274,11 +302,6 @@ public class SimpleRedisOperationsSessionRepository this.delta.clear(); } - private void putAttribute(String name, Object value) { - this.delta.put(name, value); - flushIfRequired(); - } - } } diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/EnableRedisHttpSession.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/EnableRedisHttpSession.java index 3f8facb5..1b22458e 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/EnableRedisHttpSession.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/EnableRedisHttpSession.java @@ -27,6 +27,7 @@ import org.springframework.context.annotation.Import; import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.session.SessionRepository; import org.springframework.session.config.annotation.web.http.EnableSpringHttpSession; @@ -119,4 +120,12 @@ public @interface EnableRedisHttpSession { */ String cleanupCron() default RedisHttpSessionConfiguration.DEFAULT_CLEANUP_CRON; + /** + * Save mode for the session. The default is {@link SaveMode#ON_SET_ATTRIBUTE}, which + * only saves changes made to session. + * @return the save mode + * @since 2.2.0 + */ + SaveMode saveMode() default SaveMode.ON_SET_ATTRIBUTE; + } diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfiguration.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfiguration.java index e3c4e6a1..8b55db36 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfiguration.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfiguration.java @@ -50,6 +50,7 @@ import org.springframework.scheduling.annotation.SchedulingConfigurer; import org.springframework.scheduling.config.ScheduledTaskRegistrar; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.config.annotation.web.http.SpringHttpSessionConfiguration; import org.springframework.session.data.redis.RedisFlushMode; import org.springframework.session.data.redis.RedisOperationsSessionRepository; @@ -86,6 +87,8 @@ public class RedisHttpSessionConfiguration extends SpringHttpSessionConfiguratio private FlushMode flushMode = FlushMode.ON_SAVE; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + private String cleanupCron = DEFAULT_CLEANUP_CRON; private ConfigureRedisAction configureRedisAction = new ConfigureNotifyKeyspaceEventsAction(); @@ -117,6 +120,7 @@ public class RedisHttpSessionConfiguration extends SpringHttpSessionConfiguratio sessionRepository.setRedisKeyNamespace(this.redisNamespace); } sessionRepository.setFlushMode(this.flushMode); + sessionRepository.setSaveMode(this.saveMode); int database = resolveDatabase(); sessionRepository.setDatabase(database); return sessionRepository; @@ -165,6 +169,10 @@ public class RedisHttpSessionConfiguration extends SpringHttpSessionConfiguratio this.flushMode = flushMode; } + public void setSaveMode(SaveMode saveMode) { + this.saveMode = saveMode; + } + public void setCleanupCron(String cleanupCron) { this.cleanupCron = cleanupCron; } @@ -240,6 +248,7 @@ public class RedisHttpSessionConfiguration extends SpringHttpSessionConfiguratio flushMode = redisFlushMode.getFlushMode(); } this.flushMode = flushMode; + this.saveMode = attributes.getEnum("saveMode"); String cleanupCron = attributes.getString("cleanupCron"); if (StringUtils.hasText(cleanupCron)) { this.cleanupCron = cleanupCron; diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/EnableRedisWebSession.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/EnableRedisWebSession.java index 846f637f..39766e47 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/EnableRedisWebSession.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/EnableRedisWebSession.java @@ -27,6 +27,7 @@ import org.springframework.context.annotation.Import; import org.springframework.data.redis.connection.ReactiveRedisConnectionFactory; import org.springframework.session.MapSession; import org.springframework.session.ReactiveSessionRepository; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.session.config.annotation.web.server.EnableSpringWebSession; import org.springframework.session.data.redis.ReactiveRedisOperationsSessionRepository; @@ -98,4 +99,12 @@ public @interface EnableRedisWebSession { @Deprecated RedisFlushMode redisFlushMode() default RedisFlushMode.ON_SAVE; + /** + * Save mode for the session. The default is {@link SaveMode#ON_SET_ATTRIBUTE}, which + * only saves changes made to session. + * @return the save mode + * @since 2.2.0 + */ + SaveMode saveMode() default SaveMode.ON_SET_ATTRIBUTE; + } diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfiguration.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfiguration.java index 09b04d7a..937d630f 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfiguration.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfiguration.java @@ -35,6 +35,7 @@ import org.springframework.data.redis.serializer.RedisSerializationContext; import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.config.annotation.web.server.SpringWebSessionConfiguration; import org.springframework.session.data.redis.ReactiveRedisOperationsSessionRepository; import org.springframework.session.data.redis.RedisFlushMode; @@ -61,6 +62,8 @@ public class RedisWebSessionConfiguration extends SpringWebSessionConfiguration private String redisNamespace = ReactiveRedisOperationsSessionRepository.DEFAULT_NAMESPACE; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + private ReactiveRedisConnectionFactory redisConnectionFactory; private RedisSerializer defaultRedisSerializer; @@ -78,6 +81,7 @@ public class RedisWebSessionConfiguration extends SpringWebSessionConfiguration if (StringUtils.hasText(this.redisNamespace)) { sessionRepository.setRedisKeyNamespace(this.redisNamespace); } + sessionRepository.setSaveMode(this.saveMode); return sessionRepository; } @@ -94,6 +98,10 @@ public class RedisWebSessionConfiguration extends SpringWebSessionConfiguration Assert.notNull(redisFlushMode, "redisFlushMode cannot be null"); } + public void setSaveMode(SaveMode saveMode) { + this.saveMode = saveMode; + } + @Autowired public void setRedisConnectionFactory( @SpringSessionRedisConnectionFactory ObjectProvider springSessionRedisConnectionFactory, @@ -132,6 +140,7 @@ public class RedisWebSessionConfiguration extends SpringWebSessionConfiguration if (StringUtils.hasText(redisNamespaceValue)) { this.redisNamespace = this.embeddedValueResolver.resolveStringValue(redisNamespaceValue); } + this.saveMode = attributes.getEnum("saveMode"); } private ReactiveRedisTemplate createReactiveRedisTemplate() { diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryTests.java index d26627f8..d8d45b0d 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryTests.java @@ -32,6 +32,7 @@ import reactor.test.StepVerifier; import org.springframework.data.redis.core.ReactiveHashOperations; import org.springframework.data.redis.core.ReactiveRedisOperations; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.data.redis.ReactiveRedisOperationsSessionRepository.RedisSession; import org.springframework.test.util.ReflectionTestUtils; @@ -130,7 +131,7 @@ class ReactiveRedisOperationsSessionRepositoryTests { given(this.hashOperations.putAll(anyString(), any())).willReturn(Mono.just(true)); given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); - RedisSession newSession = this.repository.new RedisSession(); + RedisSession newSession = this.repository.new RedisSession(new MapSession(), true); StepVerifier.create(this.repository.save(newSession)).verifyComplete(); verify(this.redisOperations).opsForHash(); @@ -154,7 +155,7 @@ class ReactiveRedisOperationsSessionRepositoryTests { given(this.redisOperations.hasKey(anyString())).willReturn(Mono.just(true)); given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); - RedisSession session = this.repository.new RedisSession(new MapSession(this.cached)); + RedisSession session = this.repository.new RedisSession(this.cached, false); StepVerifier.create(this.repository.save(session)).verifyComplete(); @@ -170,7 +171,7 @@ class ReactiveRedisOperationsSessionRepositoryTests { given(this.hashOperations.putAll(anyString(), any())).willReturn(Mono.just(true)); given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); - RedisSession session = this.repository.new RedisSession(this.cached); + RedisSession session = this.repository.new RedisSession(this.cached, false); session.setLastAccessedTime(Instant.ofEpochMilli(12345678L)); StepVerifier.create(this.repository.save(session)).verifyComplete(); @@ -193,7 +194,7 @@ class ReactiveRedisOperationsSessionRepositoryTests { given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); String attrName = "attrName"; - RedisSession session = this.repository.new RedisSession(this.cached); + RedisSession session = this.repository.new RedisSession(this.cached, false); session.setAttribute(attrName, "attrValue"); StepVerifier.create(this.repository.save(session)).verifyComplete(); @@ -216,7 +217,7 @@ class ReactiveRedisOperationsSessionRepositoryTests { given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); String attrName = "attrName"; - RedisSession session = this.repository.new RedisSession(new MapSession()); + RedisSession session = this.repository.new RedisSession(new MapSession(), false); session.removeAttribute(attrName); StepVerifier.create(this.repository.save(session)).verifyComplete(); @@ -234,7 +235,7 @@ class ReactiveRedisOperationsSessionRepositoryTests { @Test void redisSessionGetAttributes() { String attrName = "attrName"; - RedisSession session = this.repository.new RedisSession(this.cached); + RedisSession session = this.repository.new RedisSession(this.cached, false); assertThat(session.getAttributeNames()).isEmpty(); session.setAttribute(attrName, "attrValue"); @@ -325,7 +326,7 @@ class ReactiveRedisOperationsSessionRepositoryTests { @Test // gh-1120 void getAttributeNamesAndRemove() { - RedisSession session = this.repository.new RedisSession(this.cached); + RedisSession session = this.repository.new RedisSession(this.cached, false); session.setAttribute("attribute1", "value1"); session.setAttribute("attribute2", "value2"); @@ -336,6 +337,78 @@ class ReactiveRedisOperationsSessionRepositoryTests { assertThat(session.getAttributeNames()).isEmpty(); } + @Test + void saveWithSaveModeOnSetAttribute() { + given(this.redisOperations.hasKey(anyString())).willReturn(Mono.just(true)); + given(this.redisOperations.opsForHash()).willReturn(this.hashOperations); + given(this.hashOperations.putAll(anyString(), any())).willReturn(Mono.just(true)); + given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); + this.repository.setSaveMode(SaveMode.ON_SET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + RedisSession session = this.repository.new RedisSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + StepVerifier.create(this.repository.save(session)).verifyComplete(); + verify(this.redisOperations).hasKey(anyString()); + verify(this.redisOperations).opsForHash(); + verify(this.hashOperations).putAll(anyString(), this.delta.capture()); + assertThat(this.delta.getValue()).hasSize(1); + verify(this.redisOperations).expire(anyString(), any()); + verifyZeroInteractions(this.redisOperations); + verifyZeroInteractions(this.hashOperations); + } + + @Test + void saveWithSaveModeOnGetAttribute() { + given(this.redisOperations.hasKey(anyString())).willReturn(Mono.just(true)); + given(this.redisOperations.opsForHash()).willReturn(this.hashOperations); + given(this.hashOperations.putAll(anyString(), any())).willReturn(Mono.just(true)); + given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); + this.repository.setSaveMode(SaveMode.ON_GET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + RedisSession session = this.repository.new RedisSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + StepVerifier.create(this.repository.save(session)).verifyComplete(); + verify(this.redisOperations).hasKey(anyString()); + verify(this.redisOperations).opsForHash(); + verify(this.hashOperations).putAll(anyString(), this.delta.capture()); + assertThat(this.delta.getValue()).hasSize(2); + verify(this.redisOperations).expire(anyString(), any()); + verifyZeroInteractions(this.redisOperations); + verifyZeroInteractions(this.hashOperations); + } + + @Test + void saveWithSaveModeAlways() { + given(this.redisOperations.hasKey(anyString())).willReturn(Mono.just(true)); + given(this.redisOperations.opsForHash()).willReturn(this.hashOperations); + given(this.hashOperations.putAll(anyString(), any())).willReturn(Mono.just(true)); + given(this.redisOperations.expire(anyString(), any())).willReturn(Mono.just(true)); + this.repository.setSaveMode(SaveMode.ALWAYS); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + RedisSession session = this.repository.new RedisSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + StepVerifier.create(this.repository.save(session)).verifyComplete(); + verify(this.redisOperations).hasKey(anyString()); + verify(this.redisOperations).opsForHash(); + verify(this.hashOperations).putAll(anyString(), this.delta.capture()); + assertThat(this.delta.getValue()).hasSize(3); + verify(this.redisOperations).expire(anyString(), any()); + verifyZeroInteractions(this.redisOperations); + verifyZeroInteractions(this.hashOperations); + } + private Map map(Object... objects) { Map result = new HashMap<>(); if (objects == null) { diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java index edc47632..74ab7c95 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java @@ -48,6 +48,7 @@ import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.session.FindByIndexNameSessionRepository; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.session.data.redis.RedisOperationsSessionRepository.RedisSession; import org.springframework.session.events.AbstractSessionEvent; @@ -146,7 +147,7 @@ class RedisOperationsSessionRepositoryTests { given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); given(this.redisOperations.boundValueOps(anyString())).willReturn(this.boundValueOperations); - RedisSession session = this.redisRepository.new RedisSession(this.cached); + RedisSession session = this.redisRepository.new RedisSession(this.cached, false); session.setLastAccessedTime(session.getLastAccessedTime()); String originalId = session.getId(); String changeSessionId = session.changeSessionId(); @@ -194,7 +195,7 @@ class RedisOperationsSessionRepositoryTests { // gh-467 @Test void saveSessionNothingChanged() { - RedisSession session = this.redisRepository.new RedisSession(this.cached); + RedisSession session = this.redisRepository.new RedisSession(this.cached, false); this.redisRepository.save(session); @@ -230,7 +231,7 @@ class RedisOperationsSessionRepositoryTests { @Test void saveJavadoc() { - RedisSession session = this.redisRepository.new RedisSession(this.cached); + RedisSession session = this.redisRepository.new RedisSession(this.cached, false); session.setLastAccessedTime(session.getLastAccessedTime()); given(this.redisOperations.boundHashOps("spring:session:sessions:session-id")) @@ -252,7 +253,7 @@ class RedisOperationsSessionRepositoryTests { @Test void saveLastAccessChanged() { - RedisSession session = this.redisRepository.new RedisSession(new MapSession(this.cached)); + RedisSession session = this.redisRepository.new RedisSession(this.cached, false); session.setLastAccessedTime(Instant.ofEpochMilli(12345678L)); given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); @@ -267,7 +268,7 @@ class RedisOperationsSessionRepositoryTests { @Test void saveSetAttribute() { String attrName = "attrName"; - RedisSession session = this.redisRepository.new RedisSession(new MapSession()); + RedisSession session = this.redisRepository.new RedisSession(new MapSession(), false); session.setAttribute(attrName, "attrValue"); given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); @@ -282,7 +283,7 @@ class RedisOperationsSessionRepositoryTests { @Test void saveRemoveAttribute() { String attrName = "attrName"; - RedisSession session = this.redisRepository.new RedisSession(new MapSession()); + RedisSession session = this.redisRepository.new RedisSession(new MapSession(), false); session.removeAttribute(attrName); given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); @@ -295,7 +296,7 @@ class RedisOperationsSessionRepositoryTests { @Test void saveExpired() { - RedisSession session = this.redisRepository.new RedisSession(new MapSession()); + RedisSession session = this.redisRepository.new RedisSession(new MapSession(), false); session.setMaxInactiveInterval(Duration.ZERO); given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); @@ -310,8 +311,7 @@ class RedisOperationsSessionRepositoryTests { @Test void redisSessionGetAttributes() { String attrName = "attrName"; - RedisSession session = this.redisRepository.new RedisSession( - Duration.ofSeconds(MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS)); + RedisSession session = this.redisRepository.createSession(); assertThat(session.getAttributeNames()).isEmpty(); session.setAttribute(attrName, "attrValue"); assertThat(session.getAttributeNames()).containsOnly(attrName); @@ -746,7 +746,7 @@ class RedisOperationsSessionRepositoryTests { void changeRedisNamespace() { String namespace = "foo:bar"; this.redisRepository.setRedisKeyNamespace(namespace); - RedisSession session = this.redisRepository.new RedisSession(new MapSession()); + RedisSession session = this.redisRepository.new RedisSession(new MapSession(), false); session.setMaxInactiveInterval(Duration.ZERO); given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); @@ -772,7 +772,7 @@ class RedisOperationsSessionRepositoryTests { @Test // gh-1120 void getAttributeNamesAndRemove() { - RedisSession session = this.redisRepository.new RedisSession(this.cached); + RedisSession session = this.redisRepository.new RedisSession(this.cached, false); session.setAttribute("attribute1", "value1"); session.setAttribute("attribute2", "value2"); @@ -836,6 +836,57 @@ class RedisOperationsSessionRepositoryTests { verifyZeroInteractions(this.publisher); } + @Test + void saveWithSaveModeOnSetAttribute() { + given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); + given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); + given(this.redisOperations.boundValueOps(anyString())).willReturn(this.boundValueOperations); + this.redisRepository.setSaveMode(SaveMode.ON_SET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + RedisSession session = this.redisRepository.new RedisSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.redisRepository.save(session); + assertThat(getDelta()).hasSize(1); + } + + @Test + void saveWithSaveModeOnGetAttribute() { + given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); + given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); + given(this.redisOperations.boundValueOps(anyString())).willReturn(this.boundValueOperations); + this.redisRepository.setSaveMode(SaveMode.ON_GET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + RedisSession session = this.redisRepository.new RedisSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.redisRepository.save(session); + assertThat(getDelta()).hasSize(2); + } + + @Test + void saveWithSaveModeAlways() { + given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); + given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); + given(this.redisOperations.boundValueOps(anyString())).willReturn(this.boundValueOperations); + this.redisRepository.setSaveMode(SaveMode.ALWAYS); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + RedisSession session = this.redisRepository.new RedisSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.redisRepository.save(session); + assertThat(getDelta()).hasSize(3); + } + private String getKey(String id) { return "spring:session:sessions:" + id; } diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepositoryTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepositoryTests.java index 65a27f9c..012eeef1 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepositoryTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/SimpleRedisOperationsSessionRepositoryTests.java @@ -35,6 +35,7 @@ import org.springframework.data.redis.core.HashOperations; import org.springframework.data.redis.core.RedisOperations; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.data.redis.SimpleRedisOperationsSessionRepository.RedisSession; import org.springframework.test.util.ReflectionTestUtils; @@ -125,6 +126,19 @@ class SimpleRedisOperationsSessionRepositoryTests { .withMessage("flushMode must not be null"); } + @Test + void setSaveMode_ValidSaveMode_ShouldSetSaveMode() { + this.sessionRepository.setSaveMode(SaveMode.ON_GET_ATTRIBUTE); + assertThat(ReflectionTestUtils.getField(this.sessionRepository, "saveMode")) + .isEqualTo(SaveMode.ON_GET_ATTRIBUTE); + } + + @Test + void setSaveMode_NullSaveMode_ShouldThrowException() { + assertThatIllegalArgumentException().isThrownBy(() -> this.sessionRepository.setSaveMode(null)) + .withMessage("saveMode must not be null"); + } + @Test void createSession_DefaultMaxInactiveInterval_ShouldCreateSession() { RedisSession redisSession = this.sessionRepository.createSession(); @@ -223,6 +237,69 @@ class SimpleRedisOperationsSessionRepositoryTests { verifyNoMoreInteractions(this.sessionHashOperations); } + @Test + void save_WithSaveModeOnSetAttribute_SholdSaveSession() { + given(this.sessionRedisOperations.hasKey(eq(TEST_SESSION_KEY))).willReturn(true); + this.sessionRepository.setSaveMode(SaveMode.ON_SET_ATTRIBUTE); + Map attributes = new HashMap<>(); + attributes.put("attribute1", "value1"); + attributes.put("attribute2", "value2"); + attributes.put("attribute3", "value3"); + RedisSession session = createTestSession(attributes); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.sessionRepository.save(session); + verify(this.sessionRedisOperations).hasKey(eq(TEST_SESSION_KEY)); + verify(this.sessionRedisOperations).opsForHash(); + verify(this.sessionRedisOperations).expireAt(eq(TEST_SESSION_KEY), eq(getExpiry(session))); + verify(this.sessionHashOperations).putAll(eq(TEST_SESSION_KEY), this.delta.capture()); + assertThat(this.delta.getValue()).hasSize(1); + verifyNoMoreInteractions(this.sessionRedisOperations); + verifyNoMoreInteractions(this.sessionHashOperations); + } + + @Test + void saveWithSaveModeOnGetAttribute() { + given(this.sessionRedisOperations.hasKey(eq(TEST_SESSION_KEY))).willReturn(true); + this.sessionRepository.setSaveMode(SaveMode.ON_GET_ATTRIBUTE); + Map attributes = new HashMap<>(); + attributes.put("attribute1", "value1"); + attributes.put("attribute2", "value2"); + attributes.put("attribute3", "value3"); + RedisSession session = createTestSession(attributes); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.sessionRepository.save(session); + verify(this.sessionRedisOperations).hasKey(eq(TEST_SESSION_KEY)); + verify(this.sessionRedisOperations).opsForHash(); + verify(this.sessionRedisOperations).expireAt(eq(TEST_SESSION_KEY), eq(getExpiry(session))); + verify(this.sessionHashOperations).putAll(eq(TEST_SESSION_KEY), this.delta.capture()); + assertThat(this.delta.getValue()).hasSize(2); + verifyNoMoreInteractions(this.sessionRedisOperations); + verifyNoMoreInteractions(this.sessionHashOperations); + } + + @Test + void saveWithSaveModeAlways() { + given(this.sessionRedisOperations.hasKey(eq(TEST_SESSION_KEY))).willReturn(true); + this.sessionRepository.setSaveMode(SaveMode.ALWAYS); + Map attributes = new HashMap<>(); + attributes.put("attribute1", "value1"); + attributes.put("attribute2", "value2"); + attributes.put("attribute3", "value3"); + RedisSession session = createTestSession(attributes); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.sessionRepository.save(session); + verify(this.sessionRedisOperations).hasKey(eq(TEST_SESSION_KEY)); + verify(this.sessionRedisOperations).opsForHash(); + verify(this.sessionRedisOperations).expireAt(eq(TEST_SESSION_KEY), eq(getExpiry(session))); + verify(this.sessionHashOperations).putAll(eq(TEST_SESSION_KEY), this.delta.capture()); + assertThat(this.delta.getValue()).hasSize(3); + verifyNoMoreInteractions(this.sessionRedisOperations); + verifyNoMoreInteractions(this.sessionHashOperations); + } + @Test void save_SessionNotExists_ShouldThrowException() { RedisSession session = createTestSession(); @@ -315,12 +392,16 @@ class SimpleRedisOperationsSessionRepositoryTests { return result; } - private RedisSession createTestSession() { + private RedisSession createTestSession(Map attributes) { MapSession cached = new MapSession(TEST_SESSION_ID); cached.setCreationTime(Instant.EPOCH); cached.setLastAccessedTime(Instant.EPOCH); - cached.setAttribute("attribute1", "value1"); - return this.sessionRepository.new RedisSession(cached); + attributes.forEach(cached::setAttribute); + return this.sessionRepository.new RedisSession(cached, false); + } + + private RedisSession createTestSession() { + return createTestSession(Collections.singletonMap("attribute1", "value1")); } } diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfigurationTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfigurationTests.java index 6bd73110..df9babe2 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfigurationTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/http/RedisHttpSessionConfigurationTests.java @@ -35,6 +35,7 @@ import org.springframework.data.redis.core.RedisOperations; import org.springframework.data.redis.listener.RedisMessageListenerContainer; import org.springframework.mock.env.MockEnvironment; import org.springframework.session.FlushMode; +import org.springframework.session.SaveMode; import org.springframework.session.data.redis.RedisFlushMode; import org.springframework.session.data.redis.RedisOperationsSessionRepository; import org.springframework.session.data.redis.config.annotation.SpringSessionRedisConnectionFactory; @@ -142,6 +143,20 @@ class RedisHttpSessionConfigurationTests { assertThat(ReflectionTestUtils.getField(configuration, "cleanupCron")).isEqualTo(CLEANUP_CRON_EXPRESSION); } + @Test + void customSaveModeAnnotation() { + registerAndRefresh(RedisConfig.class, CustomSaveModeExpressionAnnotationConfiguration.class); + assertThat(this.context.getBean(RedisOperationsSessionRepository.class)).hasFieldOrPropertyWithValue("saveMode", + SaveMode.ALWAYS); + } + + @Test + void customSaveModeSetter() { + registerAndRefresh(RedisConfig.class, CustomSaveModeExpressionSetterConfiguration.class); + assertThat(this.context.getBean(RedisOperationsSessionRepository.class)).hasFieldOrPropertyWithValue("saveMode", + SaveMode.ALWAYS); + } + @Test void qualifiedConnectionFactoryRedisConfig() { registerAndRefresh(RedisConfig.class, QualifiedConnectionFactoryRedisConfig.class); @@ -299,6 +314,20 @@ class RedisHttpSessionConfigurationTests { } + @EnableRedisHttpSession(saveMode = SaveMode.ALWAYS) + static class CustomSaveModeExpressionAnnotationConfiguration { + + } + + @Configuration + static class CustomSaveModeExpressionSetterConfiguration extends RedisHttpSessionConfiguration { + + CustomSaveModeExpressionSetterConfiguration() { + setSaveMode(SaveMode.ALWAYS); + } + + } + @Configuration @EnableRedisHttpSession static class QualifiedConnectionFactoryRedisConfig { diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfigurationTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfigurationTests.java index 7726cbac..dee91312 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfigurationTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/server/RedisWebSessionConfigurationTests.java @@ -29,6 +29,7 @@ import org.springframework.data.redis.connection.ReactiveRedisConnectionFactory; import org.springframework.data.redis.core.ReactiveRedisOperations; import org.springframework.data.redis.serializer.RedisSerializationContext; import org.springframework.data.redis.serializer.RedisSerializer; +import org.springframework.session.SaveMode; import org.springframework.session.data.redis.ReactiveRedisOperationsSessionRepository; import org.springframework.session.data.redis.config.annotation.SpringSessionRedisConnectionFactory; import org.springframework.session.data.redis.config.annotation.SpringSessionRedisOperations; @@ -107,6 +108,20 @@ class RedisWebSessionConfigurationTests { .isEqualTo(MAX_INACTIVE_INTERVAL_IN_SECONDS); } + @Test + void customSaveModeAnnotation() { + registerAndRefresh(RedisConfig.class, CustomSaveModeExpressionAnnotationConfiguration.class); + assertThat(this.context.getBean(ReactiveRedisOperationsSessionRepository.class)) + .hasFieldOrPropertyWithValue("saveMode", SaveMode.ALWAYS); + } + + @Test + void customSaveModeSetter() { + registerAndRefresh(RedisConfig.class, CustomSaveModeExpressionSetterConfiguration.class); + assertThat(this.context.getBean(ReactiveRedisOperationsSessionRepository.class)) + .hasFieldOrPropertyWithValue("saveMode", SaveMode.ALWAYS); + } + @Test void qualifiedConnectionFactoryRedisConfig() { registerAndRefresh(RedisConfig.class, QualifiedConnectionFactoryRedisConfig.class); @@ -249,6 +264,20 @@ class RedisWebSessionConfigurationTests { } + @EnableRedisWebSession(saveMode = SaveMode.ALWAYS) + static class CustomSaveModeExpressionAnnotationConfiguration { + + } + + @Configuration + static class CustomSaveModeExpressionSetterConfiguration extends RedisWebSessionConfiguration { + + CustomSaveModeExpressionSetterConfiguration() { + setSaveMode(SaveMode.ALWAYS); + } + + } + @EnableRedisWebSession static class QualifiedConnectionFactoryRedisConfig { diff --git a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java index f676b281..181ebfde 100644 --- a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java +++ b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java @@ -45,6 +45,7 @@ import org.springframework.session.FlushMode; import org.springframework.session.IndexResolver; import org.springframework.session.MapSession; import org.springframework.session.PrincipalNameIndexResolver; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.session.events.AbstractSessionEvent; import org.springframework.session.events.SessionCreatedEvent; @@ -147,6 +148,8 @@ public class HazelcastSessionRepository private FlushMode flushMode = FlushMode.ON_SAVE; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + private IMap sessions; private String sessionListenerId; @@ -220,13 +223,24 @@ public class HazelcastSessionRepository this.flushMode = flushMode; } + /** + * Set the save mode. + * @param saveMode the save mode + */ + public void setSaveMode(SaveMode saveMode) { + Assert.notNull(saveMode, "saveMode must not be null"); + this.saveMode = saveMode; + } + @Override public HazelcastSession createSession() { - HazelcastSession result = new HazelcastSession(); + MapSession cached = new MapSession(); if (this.defaultMaxInactiveInterval != null) { - result.setMaxInactiveInterval(Duration.ofSeconds(this.defaultMaxInactiveInterval)); + cached.setMaxInactiveInterval(Duration.ofSeconds(this.defaultMaxInactiveInterval)); } - return result; + HazelcastSession session = new HazelcastSession(cached, true); + session.flushImmediateIfNecessary(); + return session; } @Override @@ -253,7 +267,7 @@ public class HazelcastSessionRepository entryProcessor.setMaxInactiveInterval(session.getMaxInactiveInterval()); } if (!session.delta.isEmpty()) { - entryProcessor.setDelta(session.delta); + entryProcessor.setDelta(new HashMap<>(session.delta)); } this.sessions.executeOnKey(session.getId(), entryProcessor); } @@ -274,7 +288,7 @@ public class HazelcastSessionRepository deleteById(saved.getId()); return null; } - return new HazelcastSession(saved); + return new HazelcastSession(saved, false); } @Override @@ -290,7 +304,7 @@ public class HazelcastSessionRepository Collection sessions = this.sessions.values(Predicates.equal(PRINCIPAL_NAME_ATTRIBUTE, indexValue)); Map sessionMap = new HashMap<>(sessions.size()); for (MapSession session : sessions) { - sessionMap.put(session.getId(), new HazelcastSession(session)); + sessionMap.put(session.getId(), new HazelcastSession(session, false)); } return sessionMap; } @@ -347,25 +361,14 @@ public class HazelcastSessionRepository private Map delta = new HashMap<>(); - /** - * Creates a new instance ensuring to mark all of the new attributes to be - * persisted in the next save operation. - */ - HazelcastSession() { - this(new MapSession()); - this.isNew = true; - flushImmediateIfNecessary(); - } - - /** - * Creates a new instance from the provided {@link MapSession}. - * @param cached the {@link MapSession} that represents the persisted session that - * was retrieved. Cannot be {@code null}. - */ - HazelcastSession(MapSession cached) { - Assert.notNull(cached, "MapSession cannot be null"); + HazelcastSession(MapSession cached, boolean isNew) { this.delegate = cached; + this.isNew = isNew; this.originalId = cached.getId(); + if (this.isNew || (HazelcastSessionRepository.this.saveMode == SaveMode.ALWAYS)) { + getAttributeNames() + .forEach((attributeName) -> this.delta.put(attributeName, cached.getAttribute(attributeName))); + } } @Override @@ -416,7 +419,11 @@ public class HazelcastSessionRepository @Override public T getAttribute(String attributeName) { - return this.delegate.getAttribute(attributeName); + T attributeValue = this.delegate.getAttribute(attributeName); + if (attributeValue != null && HazelcastSessionRepository.this.saveMode.equals(SaveMode.ON_GET_ATTRIBUTE)) { + this.delta.put(attributeName, attributeValue); + } + return attributeValue; } @Override diff --git a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/EnableHazelcastHttpSession.java b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/EnableHazelcastHttpSession.java index 971ea71c..8339e1d4 100644 --- a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/EnableHazelcastHttpSession.java +++ b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/EnableHazelcastHttpSession.java @@ -28,6 +28,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.session.SessionRepository; import org.springframework.session.config.annotation.web.http.EnableSpringHttpSession; @@ -113,4 +114,12 @@ public @interface EnableHazelcastHttpSession { */ FlushMode flushMode() default FlushMode.ON_SAVE; + /** + * Save mode for the session. The default is {@link SaveMode#ON_SET_ATTRIBUTE}, which + * only saves changes made to session. + * @return the save mode + * @since 2.2.0 + */ + SaveMode saveMode() default SaveMode.ON_SET_ATTRIBUTE; + } diff --git a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfiguration.java b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfiguration.java index 96abb1d3..3141f341 100644 --- a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfiguration.java +++ b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfiguration.java @@ -30,6 +30,7 @@ import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.type.AnnotationMetadata; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.config.annotation.web.http.SpringHttpSessionConfiguration; import org.springframework.session.hazelcast.HazelcastFlushMode; import org.springframework.session.hazelcast.HazelcastSessionRepository; @@ -56,6 +57,8 @@ public class HazelcastHttpSessionConfiguration extends SpringHttpSessionConfigur private FlushMode flushMode = FlushMode.ON_SAVE; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + private HazelcastInstance hazelcastInstance; private ApplicationEventPublisher applicationEventPublisher; @@ -69,6 +72,7 @@ public class HazelcastHttpSessionConfiguration extends SpringHttpSessionConfigur } sessionRepository.setDefaultMaxInactiveInterval(this.maxInactiveIntervalInSeconds); sessionRepository.setFlushMode(this.flushMode); + sessionRepository.setSaveMode(this.saveMode); return sessionRepository; } @@ -89,6 +93,10 @@ public class HazelcastHttpSessionConfiguration extends SpringHttpSessionConfigur this.flushMode = flushMode; } + public void setSaveMode(SaveMode saveMode) { + this.saveMode = saveMode; + } + @Autowired public void setHazelcastInstance( @SpringSessionHazelcastInstance ObjectProvider springSessionHazelcastInstance, @@ -122,6 +130,7 @@ public class HazelcastHttpSessionConfiguration extends SpringHttpSessionConfigur flushMode = hazelcastFlushMode.getFlushMode(); } this.flushMode = flushMode; + this.saveMode = attributes.getEnum("saveMode"); } } diff --git a/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java b/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java index 6742abc3..947cd8d7 100644 --- a/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java +++ b/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java @@ -30,6 +30,7 @@ import com.hazelcast.map.listener.MapListener; import com.hazelcast.query.impl.predicates.EqualPredicate; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; @@ -37,7 +38,9 @@ import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.session.FindByIndexNameSessionRepository; import org.springframework.session.FlushMode; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.hazelcast.HazelcastSessionRepository.HazelcastSession; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -83,6 +86,12 @@ class HazelcastSessionRepositoryTests { .withMessage("HazelcastInstance must not be null"); } + @Test + void setSaveModeNull() { + assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setSaveMode(null)) + .withMessage("saveMode must not be null"); + } + @Test void createSessionDefaultMaxInactiveInterval() { verify(this.sessions, times(1)).addEntryListener(any(MapListener.class), anyBoolean()); @@ -401,4 +410,61 @@ class HazelcastSessionRepositoryTests { assertThat(session.getAttributeNames()).isEmpty(); } + @Test + @SuppressWarnings("unchecked") + void saveWithSaveModeOnSetAttribute() { + verify(this.sessions).addEntryListener(any(MapListener.class), anyBoolean()); + this.repository.setSaveMode(SaveMode.ON_SET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + HazelcastSession session = this.repository.new HazelcastSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.repository.save(session); + ArgumentCaptor captor = ArgumentCaptor.forClass(SessionUpdateEntryProcessor.class); + verify(this.sessions).executeOnKey(eq(session.getId()), captor.capture()); + assertThat((Map) ReflectionTestUtils.getField(captor.getValue(), "delta")).hasSize(1); + verifyZeroInteractions(this.sessions); + } + + @Test + @SuppressWarnings("unchecked") + void saveWithSaveModeOnGetAttribute() { + verify(this.sessions).addEntryListener(any(MapListener.class), anyBoolean()); + this.repository.setSaveMode(SaveMode.ON_GET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + HazelcastSession session = this.repository.new HazelcastSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.repository.save(session); + ArgumentCaptor captor = ArgumentCaptor.forClass(SessionUpdateEntryProcessor.class); + verify(this.sessions).executeOnKey(eq(session.getId()), captor.capture()); + assertThat((Map) ReflectionTestUtils.getField(captor.getValue(), "delta")).hasSize(2); + verifyZeroInteractions(this.sessions); + } + + @Test + @SuppressWarnings("unchecked") + void saveWithSaveModeAlways() { + verify(this.sessions).addEntryListener(any(MapListener.class), anyBoolean()); + this.repository.setSaveMode(SaveMode.ALWAYS); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", "value1"); + delegate.setAttribute("attribute2", "value2"); + delegate.setAttribute("attribute3", "value3"); + HazelcastSession session = this.repository.new HazelcastSession(delegate, false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.repository.save(session); + ArgumentCaptor captor = ArgumentCaptor.forClass(SessionUpdateEntryProcessor.class); + verify(this.sessions).executeOnKey(eq(session.getId()), captor.capture()); + assertThat((Map) ReflectionTestUtils.getField(captor.getValue(), "delta")).hasSize(3); + verifyZeroInteractions(this.sessions); + } + } diff --git a/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfigurationTests.java b/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfigurationTests.java index 379d951e..1c175586 100644 --- a/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfigurationTests.java +++ b/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/config/annotation/web/http/HazelcastHttpSessionConfigurationTests.java @@ -27,6 +27,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; import org.springframework.session.FlushMode; +import org.springframework.session.SaveMode; import org.springframework.session.hazelcast.HazelcastFlushMode; import org.springframework.session.hazelcast.HazelcastSessionRepository; import org.springframework.session.hazelcast.config.annotation.SpringSessionHazelcastInstance; @@ -149,6 +150,20 @@ class HazelcastHttpSessionConfigurationTests { assertThat(ReflectionTestUtils.getField(repository, "flushMode")).isEqualTo(FlushMode.IMMEDIATE); } + @Test + void customSaveModeAnnotation() { + registerAndRefresh(BaseConfiguration.class, CustomSaveModeExpressionAnnotationConfiguration.class); + assertThat(this.context.getBean(HazelcastSessionRepository.class)).hasFieldOrPropertyWithValue("saveMode", + SaveMode.ALWAYS); + } + + @Test + void customSaveModeSetter() { + registerAndRefresh(BaseConfiguration.class, CustomSaveModeExpressionSetterConfiguration.class); + assertThat(this.context.getBean(HazelcastSessionRepository.class)).hasFieldOrPropertyWithValue("saveMode", + SaveMode.ALWAYS); + } + @Test void qualifiedHazelcastInstanceConfiguration() { registerAndRefresh(QualifiedHazelcastInstanceConfiguration.class); @@ -285,6 +300,20 @@ class HazelcastHttpSessionConfigurationTests { } + @EnableHazelcastHttpSession(saveMode = SaveMode.ALWAYS) + static class CustomSaveModeExpressionAnnotationConfiguration { + + } + + @Configuration + static class CustomSaveModeExpressionSetterConfiguration extends HazelcastHttpSessionConfiguration { + + CustomSaveModeExpressionSetterConfiguration() { + setSaveMode(SaveMode.ALWAYS); + } + + } + @Configuration @EnableHazelcastHttpSession(flushMode = FlushMode.IMMEDIATE) static class CustomFlushImmediatelyConfiguration extends BaseConfiguration { diff --git a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java index a50656b1..4e86428b 100644 --- a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java +++ b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java @@ -51,6 +51,7 @@ import org.springframework.session.FindByIndexNameSessionRepository; import org.springframework.session.IndexResolver; import org.springframework.session.MapSession; import org.springframework.session.PrincipalNameIndexResolver; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; @@ -237,6 +238,8 @@ public class JdbcOperationsSessionRepository private LobHandler lobHandler = new DefaultLobHandler(); + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + /** * Create a new {@link JdbcOperationsSessionRepository} instance which uses the * provided {@link JdbcOperations} to manage sessions. @@ -383,13 +386,22 @@ public class JdbcOperationsSessionRepository this.conversionService = conversionService; } + /** + * Set the save mode. + * @param saveMode the save mode + */ + public void setSaveMode(SaveMode saveMode) { + Assert.notNull(saveMode, "saveMode must not be null"); + this.saveMode = saveMode; + } + @Override public JdbcSession createSession() { - JdbcSession session = new JdbcSession(); + MapSession delegate = new MapSession(); if (this.defaultMaxInactiveInterval != null) { - session.setMaxInactiveInterval(Duration.ofSeconds(this.defaultMaxInactiveInterval)); + delegate.setMaxInactiveInterval(Duration.ofSeconds(this.defaultMaxInactiveInterval)); } - return session; + return new JdbcSession(delegate, UUID.randomUUID().toString(), true); } @Override @@ -708,17 +720,13 @@ public class JdbcOperationsSessionRepository private Map delta = new HashMap<>(); - JdbcSession() { - this.delegate = new MapSession(); - this.isNew = true; - this.primaryKey = UUID.randomUUID().toString(); - } - - JdbcSession(String primaryKey, Session delegate) { - Assert.notNull(primaryKey, "primaryKey cannot be null"); - Assert.notNull(delegate, "Session cannot be null"); - this.primaryKey = primaryKey; + JdbcSession(MapSession delegate, String primaryKey, boolean isNew) { this.delegate = delegate; + this.primaryKey = primaryKey; + this.isNew = isNew; + if (this.isNew || (JdbcOperationsSessionRepository.this.saveMode == SaveMode.ALWAYS)) { + getAttributeNames().forEach((attributeName) -> this.delta.put(attributeName, DeltaValue.UPDATED)); + } } boolean isNew() { @@ -757,7 +765,15 @@ public class JdbcOperationsSessionRepository @Override public T getAttribute(String attributeName) { Supplier supplier = this.delegate.getAttribute(attributeName); - return (supplier != null) ? supplier.get() : null; + if (supplier == null) { + return null; + } + T attributeValue = supplier.get(); + if (attributeValue != null + && JdbcOperationsSessionRepository.this.saveMode.equals(SaveMode.ON_GET_ATTRIBUTE)) { + this.delta.put(attributeName, DeltaValue.UPDATED); + } + return attributeValue; } @Override @@ -848,7 +864,7 @@ public class JdbcOperationsSessionRepository delegate.setCreationTime(Instant.ofEpochMilli(rs.getLong("CREATION_TIME"))); delegate.setLastAccessedTime(Instant.ofEpochMilli(rs.getLong("LAST_ACCESS_TIME"))); delegate.setMaxInactiveInterval(Duration.ofSeconds(rs.getInt("MAX_INACTIVE_INTERVAL"))); - session = new JdbcSession(primaryKey, delegate); + session = new JdbcSession(delegate, primaryKey, false); } String attributeName = rs.getString("ATTRIBUTE_NAME"); if (attributeName != null) { diff --git a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/EnableJdbcHttpSession.java b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/EnableJdbcHttpSession.java index d5b2832f..dacada0d 100644 --- a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/EnableJdbcHttpSession.java +++ b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/EnableJdbcHttpSession.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2017 the original author or authors. + * Copyright 2014-2019 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. @@ -27,6 +27,7 @@ import javax.sql.DataSource; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.config.annotation.web.http.EnableSpringHttpSession; import org.springframework.session.jdbc.JdbcOperationsSessionRepository; import org.springframework.session.web.http.SessionRepositoryFilter; @@ -96,4 +97,12 @@ public @interface EnableJdbcHttpSession { */ String cleanupCron() default JdbcHttpSessionConfiguration.DEFAULT_CLEANUP_CRON; + /** + * Save mode for the session. The default is {@link SaveMode#ON_SET_ATTRIBUTE}, which + * only saves changes made to session. + * @return the save mode + * @since 2.2.0 + */ + SaveMode saveMode() default SaveMode.ON_SET_ATTRIBUTE; + } diff --git a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java index a4ee8f7b..41f75cb4 100644 --- a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java +++ b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java @@ -43,6 +43,7 @@ import org.springframework.scheduling.annotation.EnableScheduling; import org.springframework.scheduling.annotation.SchedulingConfigurer; import org.springframework.scheduling.config.ScheduledTaskRegistrar; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.config.annotation.web.http.SpringHttpSessionConfiguration; import org.springframework.session.jdbc.JdbcOperationsSessionRepository; import org.springframework.session.jdbc.config.annotation.SpringSessionDataSource; @@ -77,6 +78,8 @@ public class JdbcHttpSessionConfiguration extends SpringHttpSessionConfiguration private String cleanupCron = DEFAULT_CLEANUP_CRON; + private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE; + private DataSource dataSource; private PlatformTransactionManager transactionManager; @@ -100,6 +103,7 @@ public class JdbcHttpSessionConfiguration extends SpringHttpSessionConfiguration sessionRepository.setTableName(this.tableName); } sessionRepository.setDefaultMaxInactiveInterval(this.maxInactiveIntervalInSeconds); + sessionRepository.setSaveMode(this.saveMode); if (this.lobHandler != null) { sessionRepository.setLobHandler(this.lobHandler); } @@ -142,6 +146,10 @@ public class JdbcHttpSessionConfiguration extends SpringHttpSessionConfiguration this.cleanupCron = cleanupCron; } + public void setSaveMode(SaveMode saveMode) { + this.saveMode = saveMode; + } + @Autowired public void setDataSource(@SpringSessionDataSource ObjectProvider springSessionDataSource, ObjectProvider dataSource) { @@ -199,6 +207,7 @@ public class JdbcHttpSessionConfiguration extends SpringHttpSessionConfiguration if (StringUtils.hasText(cleanupCron)) { this.cleanupCron = cleanupCron; } + this.saveMode = attributes.getEnum("saveMode"); } @Override diff --git a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java index 4ff240af..c616cef4 100644 --- a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java +++ b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java @@ -22,6 +22,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.UUID; +import java.util.function.Supplier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -36,6 +38,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.session.FindByIndexNameSessionRepository; import org.springframework.session.MapSession; +import org.springframework.session.SaveMode; import org.springframework.session.Session; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; @@ -224,6 +227,12 @@ class JdbcOperationsSessionRepositoryTests { .withMessage("conversionService must not be null"); } + @Test + void setSaveModeNull() { + assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setSaveMode(null)) + .withMessage("saveMode must not be null"); + } + @Test void createSessionDefaultMaxInactiveInterval() { JdbcOperationsSessionRepository.JdbcSession session = this.repository.createSession(); @@ -292,8 +301,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedAddSingleAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName", "testValue"); this.repository.save(session); @@ -307,8 +316,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedAddMultipleAttributes() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName1", "testValue1"); session.setAttribute("testName2", "testValue2"); @@ -323,8 +332,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedModifySingleAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName", "testValue"); session.clearChangeFlags(); session.setAttribute("testName", "testValue"); @@ -340,8 +349,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedModifyMultipleAttributes() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName1", "testValue1"); session.setAttribute("testName2", "testValue2"); session.clearChangeFlags(); @@ -359,8 +368,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedRemoveSingleAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName", "testValue"); session.clearChangeFlags(); session.removeAttribute("testName"); @@ -376,8 +385,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedRemoveNonExistingAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.removeAttribute("testName"); this.repository.save(session); @@ -389,8 +398,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedRemoveMultipleAttributes() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName1", "testValue1"); session.setAttribute("testName2", "testValue2"); session.clearChangeFlags(); @@ -408,8 +417,8 @@ class JdbcOperationsSessionRepositoryTests { @Test // gh-1070 void saveUpdatedAddAndModifyAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName", "testValue1"); session.setAttribute("testName", "testValue2"); @@ -424,8 +433,8 @@ class JdbcOperationsSessionRepositoryTests { @Test // gh-1070 void saveUpdatedAddAndRemoveAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName", "testValue"); session.removeAttribute("testName"); @@ -438,8 +447,8 @@ class JdbcOperationsSessionRepositoryTests { @Test // gh-1070 void saveUpdatedModifyAndRemoveAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName", "testValue1"); session.clearChangeFlags(); session.setAttribute("testName", "testValue2"); @@ -456,8 +465,8 @@ class JdbcOperationsSessionRepositoryTests { @Test // gh-1070 void saveUpdatedRemoveAndAddAttribute() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setAttribute("testName", "testValue1"); session.clearChangeFlags(); session.removeAttribute("testName"); @@ -474,8 +483,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedLastAccessedTime() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setLastAccessedTime(Instant.now()); this.repository.save(session); @@ -489,8 +498,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUnchanged() { - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); this.repository.save(session); @@ -516,7 +525,7 @@ class JdbcOperationsSessionRepositoryTests { @Test @SuppressWarnings("unchecked") void getSessionExpired() { - Session expired = this.repository.new JdbcSession(); + Session expired = this.repository.createSession(); expired.setLastAccessedTime(Instant.now().minusSeconds(MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS + 1)); given(this.jdbcOperations.query(isA(String.class), isA(PreparedStatementSetter.class), isA(ResultSetExtractor.class))).willReturn(Collections.singletonList(expired)); @@ -533,7 +542,7 @@ class JdbcOperationsSessionRepositoryTests { @Test @SuppressWarnings("unchecked") void getSessionFound() { - Session saved = this.repository.new JdbcSession("primaryKey", new MapSession()); + Session saved = this.repository.new JdbcSession(new MapSession(), "primaryKey", false); saved.setAttribute("savedName", "savedValue"); given(this.jdbcOperations.query(isA(String.class), isA(PreparedStatementSetter.class), isA(ResultSetExtractor.class))).willReturn(Collections.singletonList(saved)); @@ -592,10 +601,10 @@ class JdbcOperationsSessionRepositoryTests { Authentication authentication = new UsernamePasswordAuthenticationToken(principal, "notused", AuthorityUtils.createAuthorityList("ROLE_USER")); List saved = new ArrayList<>(2); - Session saved1 = this.repository.new JdbcSession(); + Session saved1 = this.repository.createSession(); saved1.setAttribute(SPRING_SECURITY_CONTEXT, authentication); saved.add(saved1); - Session saved2 = this.repository.new JdbcSession(); + Session saved2 = this.repository.createSession(); saved2.setAttribute(SPRING_SECURITY_CONTEXT, authentication); saved.add(saved2); given(this.jdbcOperations.query(isA(String.class), isA(PreparedStatementSetter.class), @@ -647,8 +656,8 @@ class JdbcOperationsSessionRepositoryTests { @Test void saveUpdatedWithoutTransaction() { this.repository = new JdbcOperationsSessionRepository(this.jdbcOperations); - JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession("primaryKey", - new MapSession()); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(new MapSession(), + "primaryKey", false); session.setLastAccessedTime(Instant.now()); this.repository.save(session); @@ -709,6 +718,61 @@ class JdbcOperationsSessionRepositoryTests { verifyZeroInteractions(this.transactionManager); } + @Test + void saveWithSaveModeOnSetAttribute() { + this.repository.setSaveMode(SaveMode.ON_SET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", (Supplier) () -> "value1"); + delegate.setAttribute("attribute2", (Supplier) () -> "value2"); + delegate.setAttribute("attribute3", (Supplier) () -> "value3"); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(delegate, + UUID.randomUUID().toString(), false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.repository.save(session); + verify(this.jdbcOperations).update(startsWith("UPDATE SPRING_SESSION_ATTRIBUTES SET"), + isA(PreparedStatementSetter.class)); + verifyZeroInteractions(this.jdbcOperations); + } + + @Test + void saveWithSaveModeOnGetAttribute() { + this.repository.setSaveMode(SaveMode.ON_GET_ATTRIBUTE); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", (Supplier) () -> "value1"); + delegate.setAttribute("attribute2", (Supplier) () -> "value2"); + delegate.setAttribute("attribute3", (Supplier) () -> "value3"); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(delegate, + UUID.randomUUID().toString(), false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.repository.save(session); + ArgumentCaptor captor = ArgumentCaptor + .forClass(BatchPreparedStatementSetter.class); + verify(this.jdbcOperations).batchUpdate(startsWith("UPDATE SPRING_SESSION_ATTRIBUTES SET"), captor.capture()); + assertThat(captor.getValue().getBatchSize()).isEqualTo(2); + verifyZeroInteractions(this.jdbcOperations); + } + + @Test + void saveWithSaveModeAlways() { + this.repository.setSaveMode(SaveMode.ALWAYS); + MapSession delegate = new MapSession(); + delegate.setAttribute("attribute1", (Supplier) () -> "value1"); + delegate.setAttribute("attribute2", (Supplier) () -> "value2"); + delegate.setAttribute("attribute3", (Supplier) () -> "value3"); + JdbcOperationsSessionRepository.JdbcSession session = this.repository.new JdbcSession(delegate, + UUID.randomUUID().toString(), false); + session.getAttribute("attribute2"); + session.setAttribute("attribute3", "value4"); + this.repository.save(session); + ArgumentCaptor captor = ArgumentCaptor + .forClass(BatchPreparedStatementSetter.class); + verify(this.jdbcOperations).batchUpdate(startsWith("UPDATE SPRING_SESSION_ATTRIBUTES SET"), captor.capture()); + assertThat(captor.getValue().getBatchSize()).isEqualTo(3); + verifyZeroInteractions(this.jdbcOperations); + } + private void assertPropagationRequiresNew() { ArgumentCaptor argument = ArgumentCaptor.forClass(TransactionDefinition.class); verify(this.transactionManager, atLeastOnce()).getTransaction(argument.capture()); diff --git a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfigurationTests.java b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfigurationTests.java index 3485eb5c..de1a9851 100644 --- a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfigurationTests.java +++ b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfigurationTests.java @@ -31,6 +31,7 @@ import org.springframework.core.convert.ConversionService; import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.support.lob.LobHandler; import org.springframework.mock.env.MockEnvironment; +import org.springframework.session.SaveMode; import org.springframework.session.jdbc.JdbcOperationsSessionRepository; import org.springframework.session.jdbc.config.annotation.SpringSessionDataSource; import org.springframework.test.util.ReflectionTestUtils; @@ -135,6 +136,20 @@ class JdbcHttpSessionConfigurationTests { assertThat(ReflectionTestUtils.getField(configuration, "cleanupCron")).isEqualTo(CLEANUP_CRON_EXPRESSION); } + @Test + void customSaveModeAnnotation() { + registerAndRefresh(DataSourceConfiguration.class, CustomSaveModeExpressionAnnotationConfiguration.class); + assertThat(this.context.getBean(JdbcHttpSessionConfiguration.class)).hasFieldOrPropertyWithValue("saveMode", + SaveMode.ALWAYS); + } + + @Test + void customSaveModeSetter() { + registerAndRefresh(DataSourceConfiguration.class, CustomSaveModeExpressionSetterConfiguration.class); + assertThat(this.context.getBean(JdbcHttpSessionConfiguration.class)).hasFieldOrPropertyWithValue("saveMode", + SaveMode.ALWAYS); + } + @Test void qualifiedDataSourceConfiguration() { registerAndRefresh(DataSourceConfiguration.class, QualifiedDataSourceConfiguration.class); @@ -300,6 +315,20 @@ class JdbcHttpSessionConfigurationTests { } + @EnableJdbcHttpSession(saveMode = SaveMode.ALWAYS) + static class CustomSaveModeExpressionAnnotationConfiguration { + + } + + @Configuration + static class CustomSaveModeExpressionSetterConfiguration extends JdbcHttpSessionConfiguration { + + CustomSaveModeExpressionSetterConfiguration() { + setSaveMode(SaveMode.ALWAYS); + } + + } + @EnableJdbcHttpSession static class QualifiedDataSourceConfiguration {