Skip to content

Commit 8b68f26

Browse files
marcusdacoregiomarcusdacoregio
committed
Address feedback
1 parent ee830a1 commit 8b68f26

File tree

35 files changed

+700
-253
lines changed

35 files changed

+700
-253
lines changed

‎spring-session-core/src/main/java/org/springframework/session/MapSession.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import java.util.Set;
2626
import java.util.UUID;
2727

28+
import org.springframework.util.Assert;
29+
2830
/**
2931
* <p>
3032
* A {@link Session} implementation that is backed by a {@link java.util.Map}. The
@@ -74,13 +76,27 @@ public final class MapSession implements Session, Serializable {
7476
*/
7577
private Duration maxInactiveInterval = DEFAULT_MAX_INACTIVE_INTERVAL;
7678

79+
private transient SessionIdGenerationStrategy sessionIdGenerationStrategy = UuidSessionIdGenerationStrategy
80+
.getInstance();
81+
7782
/**
7883
* Creates a new instance with a secure randomly generated identifier.
7984
*/
8085
public MapSession() {
8186
this(generateId());
8287
}
8388

89+
/**
90+
* Creates a new instance using the specified {@link SessionIdGenerationStrategy} to
91+
* generate the session id.
92+
* @param sessionIdGenerationStrategy the {@link SessionIdGenerationStrategy} to use.
93+
* @since 3.1
94+
*/
95+
public MapSession(SessionIdGenerationStrategy sessionIdGenerationStrategy) {
96+
this(sessionIdGenerationStrategy.generate());
97+
this.sessionIdGenerationStrategy = sessionIdGenerationStrategy;
98+
}
99+
84100
/**
85101
* Creates a new instance with the specified id. This is preferred to the default
86102
* constructor when the id is known to prevent unnecessary consumption on entropy
@@ -141,7 +157,7 @@ public String getOriginalId() {
141157

142158
@Override
143159
public String changeSessionId() {
144-
String changedId = generateId();
160+
String changedId = this.sessionIdGenerationStrategy.generate();
145161
setId(changedId);
146162
return changedId;
147163
}
@@ -232,6 +248,17 @@ private static String generateId() {
232248
return UUID.randomUUID().toString();
233249
}
234250

251+
/**
252+
* Sets the {@link SessionIdGenerationStrategy} to use when generating a new session
253+
* id.
254+
* @param sessionIdGenerationStrategy the {@link SessionIdGenerationStrategy} to use.
255+
* @since 3.1
256+
*/
257+
public void setSessionIdGenerationStrategy(SessionIdGenerationStrategy sessionIdGenerationStrategy) {
258+
Assert.notNull(sessionIdGenerationStrategy, "sessionIdGenerationStrategy cannot be null");
259+
this.sessionIdGenerationStrategy = sessionIdGenerationStrategy;
260+
}
261+
235262
private static final long serialVersionUID = 7160779239673823561L;
236263

237264
}

‎spring-session-core/src/main/java/org/springframework/session/MapSessionRepository.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,11 @@
3939
*/
4040
public class MapSessionRepository implements SessionRepository<MapSession> {
4141

42-
private static final SessionIdGenerationStrategy DEFAULT_STRATEGY = UuidSessionIdGenerationStrategy.getInstance();
43-
4442
private Duration defaultMaxInactiveInterval = Duration.ofSeconds(MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS);
4543

4644
private final Map<String, Session> sessions;
4745

48-
private SessionIdGenerationStrategy sessionIdGenerationStrategy = DEFAULT_STRATEGY;
46+
private SessionIdGenerationStrategy sessionIdGenerationStrategy = UuidSessionIdGenerationStrategy.getInstance();
4947

5048
/**
5149
* Creates a new instance backed by the provided {@link java.util.Map}. This allows

‎spring-session-core/src/main/java/org/springframework/session/ReactiveMapSessionRepository.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,11 @@
4141
*/
4242
public class ReactiveMapSessionRepository implements ReactiveSessionRepository<MapSession> {
4343

44-
private static final SessionIdGenerationStrategy DEFAULT_STRATEGY = UuidSessionIdGenerationStrategy.getInstance();
45-
4644
private Duration defaultMaxInactiveInterval = Duration.ofSeconds(MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS);
4745

4846
private final Map<String, Session> sessions;
4947

50-
private SessionIdGenerationStrategy sessionIdGenerationStrategy = DEFAULT_STRATEGY;
48+
private SessionIdGenerationStrategy sessionIdGenerationStrategy = UuidSessionIdGenerationStrategy.getInstance();
5149

5250
/**
5351
* Creates a new instance backed by the provided {@link Map}. This allows injecting a
@@ -88,6 +86,7 @@ public Mono<MapSession> findById(String id) {
8886
return Mono.defer(() -> Mono.justOrEmpty(this.sessions.get(id))
8987
.filter((session) -> !session.isExpired())
9088
.map(MapSession::new)
89+
.doOnNext((session) -> session.setSessionIdGenerationStrategy(this.sessionIdGenerationStrategy))
9190
.switchIfEmpty(deleteById(id).then(Mono.empty())));
9291
// @formatter:on
9392
}
@@ -100,7 +99,7 @@ public Mono<Void> deleteById(String id) {
10099
@Override
101100
public Mono<MapSession> createSession() {
102101
return Mono.defer(() -> {
103-
MapSession result = new MapSession(this.sessionIdGenerationStrategy.generate());
102+
MapSession result = new MapSession(this.sessionIdGenerationStrategy);
104103
result.setMaxInactiveInterval(this.defaultMaxInactiveInterval);
105104
return Mono.just(result);
106105
});

‎spring-session-core/src/main/java/org/springframework/session/SessionIdGenerationStrategy.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818

1919
import org.springframework.lang.NonNull;
2020

21+
/**
22+
* An interface for specifying a strategy for generating session identifiers.
23+
*
24+
* @author Marcus da Coregio
25+
* @since 3.1
26+
*/
2127
public interface SessionIdGenerationStrategy {
2228

2329
@NonNull

‎spring-session-core/src/main/java/org/springframework/session/UuidSessionIdGenerationStrategy.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@
2020

2121
import org.springframework.lang.NonNull;
2222

23+
/**
24+
* A {@link SessionIdGenerationStrategy} that generates a random UUID to be used as the
25+
* session id.
26+
*
27+
* @author Marcus da Coregio
28+
* @since 3.1
29+
*/
2330
public final class UuidSessionIdGenerationStrategy implements SessionIdGenerationStrategy {
2431

2532
private static final UuidSessionIdGenerationStrategy INSTANCE = new UuidSessionIdGenerationStrategy();
@@ -33,6 +40,10 @@ public String generate() {
3340
return UUID.randomUUID().toString();
3441
}
3542

43+
/**
44+
* Returns the singleton instance of {@link UuidSessionIdGenerationStrategy}.
45+
* @return the singleton instance of {@link UuidSessionIdGenerationStrategy}
46+
*/
3647
public static UuidSessionIdGenerationStrategy getInstance() {
3748
return INSTANCE;
3849
}

‎spring-session-core/src/test/java/org/springframework/session/MapSessionTests.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.time.Duration;
2020
import java.time.Instant;
2121
import java.util.Set;
22+
import java.util.UUID;
2223

2324
import org.junit.jupiter.api.BeforeEach;
2425
import org.junit.jupiter.api.Test;
@@ -42,6 +43,19 @@ void constructorNullSession() {
4243
.withMessage("session cannot be null");
4344
}
4445

46+
@Test
47+
void constructorWhenSessionIdGenerationStrategyThenUsesStrategy() {
48+
MapSession session = new MapSession(new FixedSessionIdGenerationStrategy("my-id"));
49+
assertThat(session.getId()).isEqualTo("my-id");
50+
}
51+
52+
@Test
53+
void constructorWhenDefaultThenUuid() {
54+
String id = this.session.getId();
55+
UUID uuid = UUID.fromString(id);
56+
assertThat(uuid).isNotNull();
57+
}
58+
4559
@Test
4660
void getAttributeWhenNullThenNull() {
4761
String result = this.session.getAttribute("attrName");
@@ -143,6 +157,41 @@ void getAttributeNamesAndRemove() {
143157
assertThat(this.session.getAttributeNames()).isEmpty();
144158
}
145159

160+
@Test
161+
void changeSessionIdWhenSessionIdStrategyThenUsesStrategy() {
162+
MapSession session = new MapSession(new IncrementalSessionIdGenerationStrategy());
163+
String idBeforeChange = session.getId();
164+
String idAfterChange = session.changeSessionId();
165+
assertThat(idBeforeChange).isEqualTo("1");
166+
assertThat(idAfterChange).isEqualTo("2");
167+
}
168+
169+
static class FixedSessionIdGenerationStrategy implements SessionIdGenerationStrategy {
170+
171+
private final String id;
172+
173+
FixedSessionIdGenerationStrategy(String id) {
174+
this.id = id;
175+
}
176+
177+
@Override
178+
public String generate() {
179+
return this.id;
180+
}
181+
182+
}
183+
184+
static class IncrementalSessionIdGenerationStrategy implements SessionIdGenerationStrategy {
185+
186+
private int counter = 1;
187+
188+
@Override
189+
public String generate() {
190+
return String.valueOf(this.counter++);
191+
}
192+
193+
}
194+
146195
static class CustomSession implements Session {
147196

148197
@Override

‎spring-session-core/src/test/java/org/springframework/session/ReactiveMapSessionRepositoryTests.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,22 @@
1919
import java.time.Duration;
2020
import java.time.Instant;
2121
import java.time.temporal.ChronoUnit;
22+
import java.util.Collections;
2223
import java.util.HashMap;
2324
import java.util.Map;
2425
import java.util.concurrent.ConcurrentHashMap;
2526

2627
import org.junit.jupiter.api.BeforeEach;
2728
import org.junit.jupiter.api.Test;
29+
import reactor.core.publisher.Mono;
30+
31+
import org.springframework.jdbc.core.PreparedStatementSetter;
32+
import org.springframework.jdbc.core.ResultSetExtractor;
2833

2934
import static org.assertj.core.api.Assertions.assertThat;
3035
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
36+
import static org.mockito.ArgumentMatchers.isA;
37+
import static org.mockito.BDDMockito.given;
3138

3239
/**
3340
* Tests for {@link ReactiveMapSessionRepository}.
@@ -152,4 +159,31 @@ void getAttributeNamesAndRemove() {
152159
assertThat(session.getAttributeNames()).isEmpty();
153160
}
154161

162+
@Test
163+
void createSessionWhenSessionIdGenerationStrategyThenUses() {
164+
this.repository.setSessionIdGenerationStrategy(() -> "test");
165+
MapSession session = this.repository.createSession().block();
166+
assertThat(session.getId()).isEqualTo("test");
167+
assertThat(session.changeSessionId()).isEqualTo("test");
168+
}
169+
170+
@Test
171+
void setSessionIdGenerationStrategyWhenNullThenThrowsException() {
172+
assertThatIllegalArgumentException().isThrownBy(() -> this.repository.setSessionIdGenerationStrategy(null))
173+
.withMessage("sessionIdGenerationStrategy cannot be null");
174+
}
175+
176+
@Test
177+
void findByIdWhenChangeSessionIdThenUsesSessionIdGenerationStrategy() {
178+
this.repository.setSessionIdGenerationStrategy(() -> "test");
179+
180+
MapSession session = this.repository.createSession().block();
181+
this.repository.save(session).block();
182+
183+
MapSession savedSession = this.repository.findById("test").block();
184+
185+
assertThat(savedSession.getId()).isEqualTo("test");
186+
assertThat(savedSession.changeSessionId()).isEqualTo("test");
187+
}
188+
155189
}

‎spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoIndexedSessionRepository.java

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ public class MongoIndexedSessionRepository
7272

7373
private static final Log logger = LogFactory.getLog(MongoIndexedSessionRepository.class);
7474

75-
private static final SessionIdGenerationStrategy DEFAULT_STRATEGY = UuidSessionIdGenerationStrategy.getInstance();
76-
7775
private final MongoOperations mongoOperations;
7876

7977
private Duration defaultMaxInactiveInterval = Duration.ofSeconds(MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS);
@@ -85,7 +83,7 @@ public class MongoIndexedSessionRepository
8583

8684
private ApplicationEventPublisher eventPublisher;
8785

88-
private SessionIdGenerationStrategy sessionIdGenerationStrategy = DEFAULT_STRATEGY;
86+
private SessionIdGenerationStrategy sessionIdGenerationStrategy = UuidSessionIdGenerationStrategy.getInstance();
8987

9088
public MongoIndexedSessionRepository(MongoOperations mongoOperations) {
9189
this.mongoOperations = mongoOperations;
@@ -94,27 +92,18 @@ public MongoIndexedSessionRepository(MongoOperations mongoOperations) {
9492
@Override
9593
public MongoSession createSession() {
9694

97-
String sessionId = this.sessionIdGenerationStrategy.generate();
98-
MongoSession session = new MongoSession(sessionId);
95+
MongoSession session = new MongoSession(this.sessionIdGenerationStrategy);
9996

10097
session.setMaxInactiveInterval(this.defaultMaxInactiveInterval);
10198

10299
publishEvent(new SessionCreatedEvent(this, session));
103100

104-
return wrapSession(session);
105-
}
106-
107-
private SessionIdGenerationStrategyAwareMongoSession wrapSession(MongoSession session) {
108-
return new SessionIdGenerationStrategyAwareMongoSession(session, this.sessionIdGenerationStrategy);
101+
return session;
109102
}
110103

111104
@Override
112105
public void save(MongoSession session) {
113-
MongoSession mongoSession = session;
114-
if (session instanceof SessionIdGenerationStrategyAwareMongoSession awareMongoSession) {
115-
mongoSession = awareMongoSession.getDelegate();
116-
}
117-
DBObject dbObject = MongoSessionUtils.convertToDBObject(this.mongoSessionConverter, mongoSession);
106+
DBObject dbObject = MongoSessionUtils.convertToDBObject(this.mongoSessionConverter, session);
118107
Assert.notNull(dbObject, "dbObject must not be null");
119108
this.mongoOperations.save(dbObject, this.collectionName);
120109
}
@@ -137,7 +126,7 @@ public MongoSession findById(String id) {
137126
deleteById(id);
138127
return null;
139128
}
140-
return wrapSession(session);
129+
session.setSessionIdGenerationStrategy(this.sessionIdGenerationStrategy);
141130
}
142131

143132
return session;
@@ -158,7 +147,8 @@ public Map<String, MongoSession> findByIndexNameAndIndexValue(String indexName,
158147
.map((query) -> this.mongoOperations.find(query, Document.class, this.collectionName))
159148
.orElse(Collections.emptyList()).stream()
160149
.map((dbSession) -> MongoSessionUtils.convertToSession(this.mongoSessionConverter, dbSession))
161-
.map(this::wrapSession).collect(Collectors.toMap(MongoSession::getId, (mapSession) -> mapSession));
150+
.peek((session) -> session.setSessionIdGenerationStrategy(this.sessionIdGenerationStrategy))
151+
.collect(Collectors.toMap(MongoSession::getId, (mapSession) -> mapSession));
162152
}
163153

164154
@Override
@@ -234,6 +224,11 @@ public void setMongoSessionConverter(final AbstractMongoSessionConverter mongoSe
234224
this.mongoSessionConverter = mongoSessionConverter;
235225
}
236226

227+
/**
228+
* Set the {@link SessionIdGenerationStrategy} to use to generate session ids.
229+
* @param sessionIdGenerationStrategy the {@link SessionIdGenerationStrategy} to use
230+
* @since 3.1
231+
*/
237232
public void setSessionIdGenerationStrategy(SessionIdGenerationStrategy sessionIdGenerationStrategy) {
238233
Assert.notNull(sessionIdGenerationStrategy, "sessionIdGenerationStrategy cannot be null");
239234
this.sessionIdGenerationStrategy = sessionIdGenerationStrategy;

0 commit comments

Comments
 (0)