Skip to content

Commit 8ad14ae

Browse files
rwinchrstoyanchev
rwinch
authored and
rstoyanchev
committed
DefaultWebSessionManager decoupled from DefaultWebSession
DefaultWebSessionManager no longer requires the WebSessionStore to use DefaultWebSession. Removed explicit start() in save(). This seemed unnecessary since at that point isStarted is guaranteed to return true. The status can be updated through the copy constructor. DefaultWebSessionTests added. Issue: SPR-15875
1 parent 8691247 commit 8ad14ae

File tree

5 files changed

+125
-128
lines changed

5 files changed

+125
-128
lines changed

‎spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -86,28 +86,9 @@ class DefaultWebSession implements WebSession {
8686
}
8787

8888
/**
89-
* Constructor to refresh an existing session for a new request.
90-
* @param existingSession the session to recreate
91-
* @param lastAccessTime the last access time
92-
* @param saveOperation save operation for the current request
93-
*/
94-
DefaultWebSession(DefaultWebSession existingSession, Instant lastAccessTime,
95-
Function<WebSession, Mono<Void>> saveOperation) {
96-
97-
this.id = existingSession.id;
98-
this.idGenerator = existingSession.idGenerator;
99-
this.attributes = existingSession.attributes;
100-
this.clock = existingSession.clock;
101-
this.changeIdOperation = existingSession.changeIdOperation;
102-
this.saveOperation = saveOperation;
103-
this.creationTime = existingSession.creationTime;
104-
this.lastAccessTime = lastAccessTime;
105-
this.maxIdleTime = existingSession.maxIdleTime;
106-
this.state = existingSession.state;
107-
}
108-
109-
/**
110-
* For testing purposes.
89+
* Constructor for creating a new session with an updated last access time.
90+
* @param existingSession the existing session to copy
91+
* @param lastAccessTime the new last access time
11192
*/
11293
DefaultWebSession(DefaultWebSession existingSession, Instant lastAccessTime) {
11394
this.id = existingSession.id;
@@ -119,7 +100,7 @@ class DefaultWebSession implements WebSession {
119100
this.creationTime = existingSession.creationTime;
120101
this.lastAccessTime = lastAccessTime;
121102
this.maxIdleTime = existingSession.maxIdleTime;
122-
this.state = existingSession.state;
103+
this.state = existingSession.isStarted() ? State.STARTED : State.NEW;
123104
}
124105

125106

‎spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
*/
1616
package org.springframework.web.server.session;
1717

18-
import java.time.Clock;
19-
import java.time.Instant;
20-
import java.time.ZoneId;
2118
import java.util.List;
2219

2320
import reactor.core.publisher.Flux;
@@ -43,9 +40,6 @@ public class DefaultWebSessionManager implements WebSessionManager {
4340

4441
private WebSessionStore sessionStore = new InMemoryWebSessionStore();
4542

46-
private Clock clock = Clock.system(ZoneId.of("GMT"));
47-
48-
4943
/**
5044
* Configure the id resolution strategy.
5145
* <p>By default an instance of {@link CookieWebSessionIdResolver}.
@@ -80,38 +74,14 @@ public WebSessionStore getSessionStore() {
8074
return this.sessionStore;
8175
}
8276

83-
/**
84-
* Configure the {@link Clock} to use to set lastAccessTime on every created
85-
* session and to calculate if it is expired.
86-
* <p>This may be useful to align to different timezone or to set the clock
87-
* back in a test, e.g. {@code Clock.offset(clock, Duration.ofMinutes(-31))}
88-
* in order to simulate session expiration.
89-
* <p>By default this is {@code Clock.system(ZoneId.of("GMT"))}.
90-
* @param clock the clock to use
91-
*/
92-
public void setClock(Clock clock) {
93-
Assert.notNull(clock, "'clock' is required.");
94-
this.clock = clock;
95-
}
96-
97-
/**
98-
* Return the configured clock for session lastAccessTime calculations.
99-
*/
100-
public Clock getClock() {
101-
return this.clock;
102-
}
103-
104-
10577
@Override
10678
public Mono<WebSession> getSession(ServerWebExchange exchange) {
10779
return Mono.defer(() ->
10880
retrieveSession(exchange)
10981
.flatMap(session -> removeSessionIfExpired(exchange, session))
11082
.flatMap(this.getSessionStore()::updateLastAccessTime)
111-
.switchIfEmpty(createSession(exchange))
112-
.cast(DefaultWebSession.class)
113-
.map(session -> new DefaultWebSession(session, session.getLastAccessTime(), s -> saveSession(exchange, s)))
114-
.doOnNext(session -> exchange.getResponse().beforeCommit(session::save)));
83+
.switchIfEmpty(this.sessionStore.createWebSession())
84+
.doOnNext(session -> exchange.getResponse().beforeCommit(() -> save(exchange, session))));
11585
}
11686

11787
private Mono<WebSession> retrieveSession(ServerWebExchange exchange) {
@@ -128,35 +98,28 @@ private Mono<WebSession> removeSessionIfExpired(ServerWebExchange exchange, WebS
12898
return Mono.just(session);
12999
}
130100

131-
private Mono<Void> saveSession(ServerWebExchange exchange, WebSession session) {
101+
private Mono<Void> save(ServerWebExchange exchange, WebSession session) {
132102
if (session.isExpired()) {
133103
return Mono.error(new IllegalStateException(
134104
"Sessions are checked for expiration and have their " +
135-
"lastAccessTime updated when first accessed during request processing. " +
136-
"However this session is expired meaning that maxIdleTime elapsed " +
137-
"before the call to session.save()."));
105+
"lastAccessTime updated when first accessed during request processing. " +
106+
"However this session is expired meaning that maxIdleTime elapsed " +
107+
"before the call to session.save()."));
138108
}
139109

140110
if (!session.isStarted()) {
141111
return Mono.empty();
142112
}
143113

144-
// Force explicit start
145-
session.start();
146-
147114
if (hasNewSessionId(exchange, session)) {
148-
this.sessionIdResolver.setSessionId(exchange, session.getId());
115+
DefaultWebSessionManager.this.sessionIdResolver.setSessionId(exchange, session.getId());
149116
}
150117

151-
return this.sessionStore.storeSession(session);
118+
return session.save();
152119
}
153120

154121
private boolean hasNewSessionId(ServerWebExchange exchange, WebSession session) {
155122
List<String> ids = getSessionIdResolver().resolveSessionIds(exchange);
156123
return ids.isEmpty() || !session.getId().equals(ids.get(0));
157124
}
158-
159-
private Mono<WebSession> createSession(ServerWebExchange exchange) {
160-
return this.sessionStore.createWebSession();
161-
}
162125
}

‎spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java

Lines changed: 37 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@
1515
*/
1616
package org.springframework.web.server.session;
1717

18-
import java.time.Clock;
19-
import java.time.Duration;
20-
import java.time.Instant;
21-
import java.time.ZoneId;
2218
import java.util.Arrays;
2319
import java.util.Collections;
2420

@@ -32,8 +28,6 @@
3228
import org.springframework.http.codec.ServerCodecConfigurer;
3329
import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
3430
import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
35-
import org.springframework.util.IdGenerator;
36-
import org.springframework.util.JdkIdGenerator;
3731
import org.springframework.web.server.ServerWebExchange;
3832
import org.springframework.web.server.WebSession;
3933
import org.springframework.web.server.adapter.DefaultServerWebExchange;
@@ -42,11 +36,11 @@
4236
import static org.junit.Assert.assertEquals;
4337
import static org.junit.Assert.assertFalse;
4438
import static org.junit.Assert.assertNotNull;
45-
import static org.junit.Assert.assertNotSame;
4639
import static org.mockito.ArgumentMatchers.any;
4740
import static org.mockito.ArgumentMatchers.eq;
4841
import static org.mockito.Mockito.never;
4942
import static org.mockito.Mockito.verify;
43+
import static org.mockito.Mockito.verifyZeroInteractions;
5044
import static org.mockito.Mockito.when;
5145

5246
/**
@@ -57,11 +51,6 @@
5751
@RunWith(MockitoJUnitRunner.class)
5852
public class DefaultWebSessionManagerTests {
5953

60-
private static final Clock CLOCK = Clock.system(ZoneId.of("GMT"));
61-
62-
private static final IdGenerator idGenerator = new JdkIdGenerator();
63-
64-
6554
private DefaultWebSessionManager manager;
6655

6756
private ServerWebExchange exchange;
@@ -72,10 +61,23 @@ public class DefaultWebSessionManagerTests {
7261
@Mock
7362
private WebSessionStore store;
7463

64+
@Mock
65+
private WebSession createSession;
66+
67+
@Mock
68+
private WebSession retrieveSession;
69+
70+
@Mock
71+
private WebSession updateSession;
72+
7573
@Before
7674
public void setUp() throws Exception {
77-
when(this.store.createWebSession()).thenReturn(Mono.just(createDefaultWebSession()));
78-
when(this.store.updateLastAccessTime(any())).thenAnswer( invocation -> Mono.just(invocation.getArgument(0)));
75+
when(this.store.createWebSession()).thenReturn(Mono.just(this.createSession));
76+
when(this.store.updateLastAccessTime(any())).thenReturn(Mono.just(this.updateSession));
77+
when(this.store.retrieveSession(any())).thenReturn(Mono.just(this.retrieveSession));
78+
when(this.createSession.save()).thenReturn(Mono.empty());
79+
when(this.updateSession.getId()).thenReturn("update-session-id");
80+
when(this.retrieveSession.getId()).thenReturn("retrieve-session-id");
7981

8082
this.manager = new DefaultWebSessionManager();
8183
this.manager.setSessionIdResolver(this.idResolver);
@@ -87,90 +89,71 @@ public void setUp() throws Exception {
8789
ServerCodecConfigurer.create(), new AcceptHeaderLocaleContextResolver());
8890
}
8991

90-
9192
@Test
92-
public void getSessionWithoutStarting() throws Exception {
93+
public void getSessionSaveWhenCreatedAndNotStartedThenNotSaved() throws Exception {
9394
when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList());
9495
WebSession session = this.manager.getSession(this.exchange).block();
95-
session.save().block();
96+
this.exchange.getResponse().setComplete().block();
9697

9798
assertFalse(session.isStarted());
9899
assertFalse(session.isExpired());
99-
verify(this.store, never()).storeSession(any());
100+
verifyZeroInteractions(this.retrieveSession, this.updateSession);
101+
verify(this.createSession, never()).save();
100102
verify(this.idResolver, never()).setSessionId(any(), any());
101103
}
102104

103105
@Test
104-
public void startSessionExplicitly() throws Exception {
106+
public void getSessionSaveWhenCreatedAndStartedThenSavesAndSetsId() throws Exception {
105107
when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList());
106-
when(this.store.storeSession(any())).thenReturn(Mono.empty());
107108
WebSession session = this.manager.getSession(this.exchange).block();
108-
session.start();
109-
session.save().block();
109+
when(this.createSession.isStarted()).thenReturn(true);
110+
this.exchange.getResponse().setComplete().block();
110111

111112
String id = session.getId();
112113
verify(this.store).createWebSession();
113-
verify(this.store).storeSession(any());
114+
verify(this.createSession).save();
114115
verify(this.idResolver).setSessionId(any(), eq(id));
115116
}
116117

117-
@Test
118-
public void startSessionImplicitly() throws Exception {
119-
when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList());
120-
when(this.store.storeSession(any())).thenReturn(Mono.empty());
121-
WebSession session = this.manager.getSession(this.exchange).block();
122-
session.getAttributes().put("foo", "bar");
123-
session.save().block();
124-
125-
verify(this.store).createWebSession();
126-
verify(this.idResolver).setSessionId(any(), any());
127-
verify(this.store).storeSession(any());
128-
}
129-
130118
@Test
131119
public void exchangeWhenResponseSetCompleteThenSavesAndSetsId() throws Exception {
132120
when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList());
133-
when(this.store.storeSession(any())).thenReturn(Mono.empty());
121+
String id = this.createSession.getId();
134122
WebSession session = this.manager.getSession(this.exchange).block();
135-
String id = session.getId();
136-
session.getAttributes().put("foo", "bar");
123+
when(this.createSession.isStarted()).thenReturn(true);
137124
this.exchange.getResponse().setComplete().block();
138125

139126
verify(this.idResolver).setSessionId(any(), eq(id));
140-
verify(this.store).storeSession(any());
127+
verify(this.createSession).save();
141128
}
142129

143130
@Test
144131
public void existingSession() throws Exception {
145-
DefaultWebSession existing = createDefaultWebSession();
146-
String id = existing.getId();
147-
when(this.store.retrieveSession(id)).thenReturn(Mono.just(existing));
132+
String id = this.updateSession.getId();
133+
when(this.store.retrieveSession(id)).thenReturn(Mono.just(this.updateSession));
148134
when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(id));
149135

150136
WebSession actual = this.manager.getSession(this.exchange).block();
151137
assertNotNull(actual);
152-
assertEquals(existing.getId(), actual.getId());
138+
assertEquals(id, actual.getId());
153139
}
154140

155141
@Test
156142
public void existingSessionIsExpired() throws Exception {
157-
DefaultWebSession existing = createDefaultWebSession();
158-
existing.start();
159-
Instant lastAccessTime = Instant.now(CLOCK).minus(Duration.ofMinutes(31));
160-
existing = new DefaultWebSession(existing, lastAccessTime, s -> Mono.empty());
161-
when(this.store.retrieveSession(existing.getId())).thenReturn(Mono.just(existing));
162-
when(this.store.removeSession(existing.getId())).thenReturn(Mono.empty());
163-
when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(existing.getId()));
143+
String id = this.retrieveSession.getId();
144+
when(this.retrieveSession.isExpired()).thenReturn(true);
145+
when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(id));
146+
when(this.store.removeSession(any())).thenReturn(Mono.empty());
164147

165148
WebSession actual = this.manager.getSession(this.exchange).block();
166-
assertNotSame(existing, actual);
167-
verify(this.store).removeSession(existing.getId());
149+
assertEquals(this.createSession.getId(), actual.getId());
150+
verify(this.store).removeSession(id);
168151
verify(this.idResolver).expireSession(any());
169152
}
170153

171154
@Test
172155
public void multipleSessionIds() throws Exception {
173-
DefaultWebSession existing = createDefaultWebSession();
156+
WebSession existing = this.updateSession;
174157
String id = existing.getId();
175158
when(this.store.retrieveSession(any())).thenReturn(Mono.empty());
176159
when(this.store.retrieveSession(id)).thenReturn(Mono.just(existing));
@@ -180,8 +163,4 @@ public void multipleSessionIds() throws Exception {
180163
assertNotNull(actual);
181164
assertEquals(existing.getId(), actual.getId());
182165
}
183-
184-
private DefaultWebSession createDefaultWebSession() {
185-
return new DefaultWebSession(idGenerator, CLOCK, (s, session) -> Mono.empty(), s -> Mono.empty());
186-
}
187166
}

0 commit comments

Comments
 (0)