Skip to content

Commit 3272917

Browse files
committed
Polish concurrency in UserSessionResolver impl
1 parent 675ec4c commit 3272917

File tree

3 files changed

+105
-20
lines changed

3 files changed

+105
-20
lines changed

spring-messaging/src/main/java/org/springframework/messaging/simp/handler/DefaultSubscriptionRegistry.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Map;
2222
import java.util.Set;
2323
import java.util.concurrent.ConcurrentHashMap;
24+
import java.util.concurrent.ConcurrentMap;
2425
import java.util.concurrent.CopyOnWriteArraySet;
2526

2627
import org.springframework.messaging.Message;
@@ -180,11 +181,9 @@ public String toString() {
180181
*/
181182
private static class SessionSubscriptionRegistry {
182183

183-
private final Map<String, SessionSubscriptionInfo> sessions =
184+
private final ConcurrentMap<String, SessionSubscriptionInfo> sessions =
184185
new ConcurrentHashMap<String, SessionSubscriptionInfo>();
185186

186-
private final Object monitor = new Object();
187-
188187

189188
public SessionSubscriptionInfo getSubscriptions(String sessionId) {
190189
return this.sessions.get(sessionId);
@@ -197,12 +196,10 @@ public Collection<SessionSubscriptionInfo> getAllSubscriptions() {
197196
public SessionSubscriptionInfo addSubscription(String sessionId, String subscriptionId, String destination) {
198197
SessionSubscriptionInfo info = this.sessions.get(sessionId);
199198
if (info == null) {
200-
synchronized(this.monitor) {
201-
info = this.sessions.get(sessionId);
202-
if (info == null) {
203-
info = new SessionSubscriptionInfo(sessionId);
204-
this.sessions.put(sessionId, info);
205-
}
199+
info = new SessionSubscriptionInfo(sessionId);
200+
SessionSubscriptionInfo value = this.sessions.putIfAbsent(sessionId, info);
201+
if (value != null) {
202+
info = value;
206203
}
207204
}
208205
info.addSubscription(destination, subscriptionId);
@@ -249,14 +246,17 @@ public Set<String> getSubscriptions(String destination) {
249246
}
250247

251248
public void addSubscription(String destination, String subscriptionId) {
252-
synchronized(this.monitor) {
253-
Set<String> subs = this.subscriptions.get(destination);
254-
if (subs == null) {
255-
subs = new HashSet<String>(4);
256-
this.subscriptions.put(destination, subs);
249+
Set<String> subs = this.subscriptions.get(destination);
250+
if (subs == null) {
251+
synchronized(this.monitor) {
252+
subs = this.subscriptions.get(destination);
253+
if (subs == null) {
254+
subs = new HashSet<String>(4);
255+
this.subscriptions.put(destination, subs);
256+
}
257257
}
258-
subs.add(subscriptionId);
259258
}
259+
subs.add(subscriptionId);
260260
}
261261

262262
public String removeSubscription(String subscriptionId) {

spring-messaging/src/main/java/org/springframework/messaging/simp/handler/SimpleUserSessionResolver.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717
package org.springframework.messaging.simp.handler;
1818

1919
import java.util.Collections;
20-
import java.util.Map;
2120
import java.util.Set;
2221
import java.util.concurrent.ConcurrentHashMap;
22+
import java.util.concurrent.ConcurrentMap;
2323
import java.util.concurrent.CopyOnWriteArraySet;
2424

2525

@@ -30,15 +30,18 @@
3030
public class SimpleUserSessionResolver implements MutableUserSessionResolver {
3131

3232
// userId -> sessionId's
33-
private final Map<String, Set<String>> userSessionIds = new ConcurrentHashMap<String, Set<String>>();
33+
private final ConcurrentMap<String, Set<String>> userSessionIds = new ConcurrentHashMap<String, Set<String>>();
3434

3535

3636
@Override
3737
public void addUserSessionId(String user, String sessionId) {
3838
Set<String> sessionIds = this.userSessionIds.get(user);
3939
if (sessionIds == null) {
4040
sessionIds = new CopyOnWriteArraySet<String>();
41-
this.userSessionIds.put(user, sessionIds);
41+
Set<String> value = this.userSessionIds.putIfAbsent(user, sessionIds);
42+
if (value != null) {
43+
sessionIds = value;
44+
}
4245
}
4346
sessionIds.add(sessionId);
4447
}
@@ -47,8 +50,8 @@ public void addUserSessionId(String user, String sessionId) {
4750
public void removeUserSessionId(String user, String sessionId) {
4851
Set<String> sessionIds = this.userSessionIds.get(user);
4952
if (sessionIds != null) {
50-
if (sessionIds.remove(sessionId) && sessionIds.isEmpty()) {
51-
this.userSessionIds.remove(user);
53+
if (sessionIds.remove(sessionId)) {
54+
this.userSessionIds.remove(user, Collections.<String>emptySet());
5255
}
5356
}
5457
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright 2002-2013 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.messaging.simp.handler;
18+
19+
import java.util.Arrays;
20+
import java.util.Collections;
21+
import java.util.LinkedHashSet;
22+
import java.util.List;
23+
24+
import org.junit.Test;
25+
26+
import static org.junit.Assert.*;
27+
28+
29+
/**
30+
* Test fixture for {@link SimpleUserSessionResolver}
31+
*
32+
* @author Rossen Stoyanchev
33+
* @since 4.0
34+
*/
35+
public class SimpleUserSessionResolverTests {
36+
37+
private static final String user = "joe";
38+
private static final List<String> sessionIds = Arrays.asList("sess01", "sess02", "sess03");
39+
40+
41+
@Test
42+
public void addOneSessionId() {
43+
44+
SimpleUserSessionResolver resolver = new SimpleUserSessionResolver();
45+
resolver.addUserSessionId(user, sessionIds.get(0));
46+
47+
assertEquals(Collections.singleton(sessionIds.get(0)), resolver.resolveUserSessionIds(user));
48+
assertSame(Collections.emptySet(), resolver.resolveUserSessionIds("jane"));
49+
}
50+
51+
@Test
52+
public void addMultipleSessionIds() {
53+
54+
SimpleUserSessionResolver resolver = new SimpleUserSessionResolver();
55+
for (String sessionId : sessionIds) {
56+
resolver.addUserSessionId(user, sessionId);
57+
}
58+
59+
assertEquals(new LinkedHashSet<>(sessionIds), resolver.resolveUserSessionIds(user));
60+
assertEquals(Collections.emptySet(), resolver.resolveUserSessionIds("jane"));
61+
}
62+
63+
64+
@Test
65+
public void removeSessionIds() {
66+
67+
SimpleUserSessionResolver resolver = new SimpleUserSessionResolver();
68+
for (String sessionId : sessionIds) {
69+
resolver.addUserSessionId(user, sessionId);
70+
}
71+
72+
assertEquals(new LinkedHashSet<>(sessionIds), resolver.resolveUserSessionIds(user));
73+
74+
resolver.removeUserSessionId(user, sessionIds.get(1));
75+
resolver.removeUserSessionId(user, sessionIds.get(2));
76+
assertEquals(Collections.singleton(sessionIds.get(0)), resolver.resolveUserSessionIds(user));
77+
78+
resolver.removeUserSessionId(user, sessionIds.get(0));
79+
assertSame(Collections.emptySet(), resolver.resolveUserSessionIds(user));
80+
}
81+
82+
}

0 commit comments

Comments
 (0)