Skip to content

Commit aaa4c62

Browse files
marcusdacoregiomarcusdacoregio
committed
Address feedback
1 parent ee830a1 commit aaa4c62

File tree

34 files changed

+664
-252
lines changed

34 files changed

+664
-252
lines changed

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

+28-1
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

+1-3
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

+1-3
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

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

+6
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 6.1
26+
*/
2127
public interface SessionIdGenerationStrategy {
2228

2329
@NonNull

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

+11
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

+49
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-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoIndexedSessionRepository.java

+12-17
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;

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

+40-3
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@
2323
import java.util.Map;
2424
import java.util.Objects;
2525
import java.util.Set;
26-
import java.util.UUID;
2726
import java.util.stream.Collectors;
2827

2928
import org.springframework.lang.Nullable;
3029
import org.springframework.session.MapSession;
3130
import org.springframework.session.Session;
31+
import org.springframework.session.SessionIdGenerationStrategy;
32+
import org.springframework.session.UuidSessionIdGenerationStrategy;
33+
import org.springframework.util.Assert;
3234

3335
/**
3436
* Session object providing additional information about the datetime of expiration.
@@ -66,6 +68,9 @@ class MongoSession implements Session {
6668

6769
private Map<String, Object> attrs = new HashMap<>();
6870

71+
private transient SessionIdGenerationStrategy sessionIdGenerationStrategy = UuidSessionIdGenerationStrategy
72+
.getInstance();
73+
6974
/**
7075
* Constructs a new instance using the provided session id.
7176
* @param sessionId the session id to use
@@ -80,7 +85,7 @@ class MongoSession implements Session {
8085
}
8186

8287
MongoSession(long maxInactiveIntervalInSeconds) {
83-
this(UUID.randomUUID().toString(), maxInactiveIntervalInSeconds);
88+
this(UuidSessionIdGenerationStrategy.getInstance().generate(), maxInactiveIntervalInSeconds);
8489
}
8590

8691
MongoSession(String id, long maxInactiveIntervalInSeconds) {
@@ -91,6 +96,28 @@ class MongoSession implements Session {
9196
setLastAccessedTimeAndUpdateExpireAt(Instant.ofEpochMilli(this.createdMillis));
9297
}
9398

99+
/**
100+
* Constructs a new instance using the provided {@link SessionIdGenerationStrategy}.
101+
* @param sessionIdGenerationStrategy the {@link SessionIdGenerationStrategy} to use
102+
* @since 3.1
103+
*/
104+
MongoSession(SessionIdGenerationStrategy sessionIdGenerationStrategy) {
105+
this(sessionIdGenerationStrategy.generate(), MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS);
106+
this.sessionIdGenerationStrategy = sessionIdGenerationStrategy;
107+
}
108+
109+
/**
110+
* Constructs a new instance using the provided {@link SessionIdGenerationStrategy}
111+
* and max inactive interval.
112+
* @param sessionIdGenerationStrategy the {@link SessionIdGenerationStrategy} to use
113+
* @param maxInactiveIntervalInSeconds the max inactive interval in seconds
114+
* @since 3.1
115+
*/
116+
MongoSession(SessionIdGenerationStrategy sessionIdGenerationStrategy, long maxInactiveIntervalInSeconds) {
117+
this(sessionIdGenerationStrategy.generate(), maxInactiveIntervalInSeconds);
118+
this.sessionIdGenerationStrategy = sessionIdGenerationStrategy;
119+
}
120+
94121
static String coverDot(String attributeName) {
95122
return attributeName.replace('.', DOT_COVER_CHAR);
96123
}
@@ -102,7 +129,7 @@ static String uncoverDot(String attributeName) {
102129
@Override
103130
public String changeSessionId() {
104131

105-
String changedId = UUID.randomUUID().toString();
132+
String changedId = this.sessionIdGenerationStrategy.generate();
106133
this.id = changedId;
107134
return changedId;
108135
}
@@ -221,4 +248,14 @@ void setId(String id) {
221248
this.id = id;
222249
}
223250

251+
/**
252+
* Sets the {@link SessionIdGenerationStrategy} to use.
253+
* @param sessionIdGenerationStrategy the {@link SessionIdGenerationStrategy} to use
254+
* @since 3.1
255+
*/
256+
void setSessionIdGenerationStrategy(SessionIdGenerationStrategy sessionIdGenerationStrategy) {
257+
Assert.notNull(sessionIdGenerationStrategy, "sessionIdGenerationStrategy cannot be null");
258+
this.sessionIdGenerationStrategy = sessionIdGenerationStrategy;
259+
}
260+
224261
}

0 commit comments

Comments
 (0)