Skip to content

Commit fb23ebf

Browse files
jxblumjxblum
committed
Refactor to make it more apparent when a race condition occurs in concurrent Session access Integration Tests.
1 parent 9aecc61 commit fb23ebf

File tree

3 files changed

+34
-21
lines changed

3 files changed

+34
-21
lines changed

‎spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractConcurrentSessionOperationsIntegrationTests.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@
3737
import org.springframework.lang.Nullable;
3838
import org.springframework.session.Session;
3939
import org.springframework.session.SessionRepository;
40+
import org.springframework.util.Assert;
4041
import org.springframework.util.ObjectUtils;
42+
import org.springframework.util.StringUtils;
4143

4244
/**
4345
* Abstract base class encapsulating functionality common to all concurrent {@link Session} data access operation
@@ -95,12 +97,23 @@ protected AbstractConcurrentSessionOperationsTestCase(
9597
return this.sessionRepository;
9698
}
9799

98-
protected @NonNull String getSessionId() {
100+
protected void setSessionId(@Nullable String sessionId) {
101+
this.sessionId.set(sessionId);
102+
}
103+
104+
protected @Nullable String getSessionId() {
99105
return this.sessionId.get();
100106
}
101107

102-
protected void setSessionId(@Nullable String sessionId) {
103-
this.sessionId.set(sessionId);
108+
protected @NonNull String requireSessionId() {
109+
110+
String sessionId = getSessionId();
111+
112+
Assert.state(StringUtils.hasText(sessionId), () -> "Session ID [%s] is not set;"
113+
+ " A race condition may have occurred between the Thread setting the Session ID and the Threads"
114+
+ " using the Session ID due to untimely operations in the Apache Geode client/server topology");
115+
116+
return sessionId;
104117
}
105118

106119
protected @Nullable Session findById(@NonNull String id) {
@@ -116,7 +129,7 @@ protected void setSessionId(@Nullable String sessionId) {
116129
return session;
117130
}
118131

119-
protected void waitOnAvailableSessionId() {
132+
protected void waitOnRequiredSessionId() {
120133
AbstractConcurrentSessionOperationsIntegrationTests.waitOn(() -> Objects.nonNull(this.sessionId.get()));
121134
}
122135
}
@@ -151,7 +164,6 @@ public void thread1() {
151164
assertThat(session.getAttributeNames()).containsOnly("attributeOne", "attributeTwo");
152165

153166
save(session);
154-
155167
setSessionId(session.getId());
156168

157169
waitForTick(4);
@@ -170,9 +182,9 @@ public void thread2() {
170182

171183
waitForTick(1);
172184
assertTick(1);
173-
waitOnAvailableSessionId();
185+
waitOnRequiredSessionId();
174186

175-
Session session = findById(getSessionId());
187+
Session session = findById(requireSessionId());
176188

177189
assertThat(session).isNotNull();
178190
assertThat(session.getId()).isEqualTo(getSessionId());
@@ -199,8 +211,9 @@ public void thread3() {
199211

200212
waitForTick(1);
201213
assertTick(1);
214+
waitOnRequiredSessionId();
202215

203-
Session session = findById(getSessionId());
216+
Session session = findById(requireSessionId());
204217

205218
assertThat(session).isNotNull();
206219
assertThat(session.getId()).isEqualTo(getSessionId());

‎spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.springframework.data.gemfire.config.annotation.CacheServerApplication;
5555
import org.springframework.data.gemfire.config.annotation.ClientCacheApplication;
5656
import org.springframework.lang.NonNull;
57+
import org.springframework.lang.Nullable;
5758
import org.springframework.session.Session;
5859
import org.springframework.session.data.gemfire.config.annotation.web.http.EnableGemFireHttpSession;
5960
import org.springframework.session.data.gemfire.config.annotation.web.http.GemFireHttpSessionConfiguration;
@@ -160,7 +161,7 @@ public void thread1() {
160161

161162
Instant beforeLastAccessedTime = Instant.now();
162163

163-
Session session = findById(getSessionId());
164+
Session session = findById(requireSessionId());
164165

165166
assertThat(session).isNotNull();
166167
assertThat(session.getId()).isEqualTo(getSessionId());
@@ -188,7 +189,7 @@ public void thread2() {
188189

189190
Instant beforeLastAccessedTime = Instant.now();
190191

191-
Session session = findById(getSessionId());
192+
Session session = findById(requireSessionId());
192193

193194
assertThat(session).isNotNull();
194195
assertThat(session.getId()).isEqualTo(getSessionId());
@@ -218,23 +219,23 @@ public static class RegionPutWithNonDirtySessionTestCase extends AbstractConcurr
218219
private final Region<Object, Session> sessions;
219220

220221
public RegionPutWithNonDirtySessionTestCase(
221-
ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests testInstance) {
222+
@NonNull ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests testInstance) {
222223

223224
super(testInstance);
224225

225226
this.sessions = testInstance.getSessionRegion();
226227
this.sessionSerializer = reregisterDataSerializer(resolveDataSerializer());
227228
}
228229

229-
private DataSerializer reregisterDataSerializer(DataSerializer dataSerializer) {
230+
private @NonNull DataSerializer reregisterDataSerializer(@NonNull DataSerializer dataSerializer) {
230231

231232
InternalDataSerializer.unregister(dataSerializer.getId());
232233
InternalDataSerializer._register(dataSerializer, false);
233234

234235
return dataSerializer;
235236
}
236237

237-
private DataSerializer resolveDataSerializer() {
238+
private @NonNull DataSerializer resolveDataSerializer() {
238239

239240
return Arrays.stream(nullSafeArray(InternalDataSerializer.getSerializers(), DataSerializer.class))
240241
.filter(this.sessionSerializerFilter())
@@ -243,7 +244,7 @@ private DataSerializer resolveDataSerializer() {
243244
.orElseThrow(() -> newIllegalStateException(DATA_SERIALIZER_NOT_FOUND_EXCEPTION_MESSAGE));
244245
}
245246

246-
private Predicate<? super DataSerializer> sessionSerializerFilter() {
247+
private @NonNull Predicate<? super DataSerializer> sessionSerializerFilter() {
247248

248249
return dataSerializer -> {
249250

@@ -260,11 +261,11 @@ private Predicate<? super DataSerializer> sessionSerializerFilter() {
260261
};
261262
}
262263

263-
private Session get(String id) {
264+
private @Nullable Session get(@NonNull String id) {
264265
return this.sessions.get(id);
265266
}
266267

267-
private void put(Session session) {
268+
private void put(@NonNull Session session) {
268269

269270
this.sessions.put(session.getId(), session);
270271

@@ -313,9 +314,9 @@ public void thread2() {
313314

314315
waitForTick(1);
315316
assertTick(1);
316-
waitOnAvailableSessionId();
317+
waitOnRequiredSessionId();
317318

318-
Session session = get(getSessionId());
319+
Session session = get(requireSessionId());
319320

320321
assertThat(session).isInstanceOf(GemFireSession.class);
321322
assertThat(session.getId()).isEqualTo(getSessionId());

‎spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientLocalRegionIntegrationTests.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ public void thread1() {
8383
assertThat(session.getAttributeNames()).isEmpty();
8484

8585
save(session);
86-
8786
setSessionId(session.getId());
8887

8988
waitForTick(2);
@@ -100,9 +99,9 @@ public void thread2() {
10099

101100
waitForTick(1);
102101
assertTick(1);
103-
waitOnAvailableSessionId();
102+
waitOnRequiredSessionId();
104103

105-
Session session = findById(getSessionId());
104+
Session session = findById(requireSessionId());
106105

107106
assertThat(session).isNotNull();
108107
assertThat(session.getId()).isEqualTo(getSessionId());

0 commit comments

Comments
 (0)