Skip to content

Commit b4e48d6

Browse files
rstoyanchevrstoyanchev
committed
Fix issue in DefaultUserDestinationResolver
DefaultUserDestinationResolver now uses the session id of SUBSCRIBE/UNSUBSCRIBE messages rather than looking up all session id's associated with a user. Issue: SPR-11325
1 parent 809a5f5 commit b4e48d6

File tree

3 files changed

+75
-48
lines changed

3 files changed

+75
-48
lines changed

‎spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -96,17 +96,13 @@ public Set<String> resolveDestination(Message<?> message) {
9696
return Collections.emptySet();
9797
}
9898

99-
Set<String> set = new HashSet<String>();
100-
for (String sessionId : this.userSessionRegistry.getSessionIds(info.getUser())) {
101-
set.add(getTargetDestination(headers.getDestination(), info.getDestination(), sessionId, info.getUser()));
99+
Set<String> result = new HashSet<String>();
100+
for (String sessionId : info.getSessionIds()) {
101+
result.add(getTargetDestination(
102+
headers.getDestination(), info.getDestination(), sessionId, info.getUser()));
102103
}
103-
return set;
104-
}
105-
106-
protected String getTargetDestination(String originalDestination, String targetDestination,
107-
String sessionId, String user) {
108104

109-
return targetDestination + "-user" + sessionId;
105+
return result;
110106
}
111107

112108
private UserDestinationInfo getUserDestinationInfo(SimpMessageHeaderAccessor headers) {
@@ -115,6 +111,7 @@ private UserDestinationInfo getUserDestinationInfo(SimpMessageHeaderAccessor hea
115111

116112
String targetUser;
117113
String targetDestination;
114+
Set<String> sessionIds;
118115

119116
Principal user = headers.getUser();
120117
SimpMessageType messageType = headers.getMessageType();
@@ -124,11 +121,16 @@ private UserDestinationInfo getUserDestinationInfo(SimpMessageHeaderAccessor hea
124121
return null;
125122
}
126123
if (user == null) {
127-
logger.warn("Ignoring message, no user information");
124+
logger.error("Ignoring message, no user info available");
125+
return null;
126+
}
127+
if (headers.getSessionId() == null) {
128+
logger.error("Ignoring message, no session id available");
128129
return null;
129130
}
130131
targetUser = user.getName();
131132
targetDestination = destination.substring(this.destinationPrefix.length()-1);
133+
sessionIds = Collections.singleton(headers.getSessionId());
132134
}
133135
else if (SimpMessageType.MESSAGE.equals(messageType)) {
134136
if (!checkDestination(destination, this.destinationPrefix)) {
@@ -139,7 +141,7 @@ else if (SimpMessageType.MESSAGE.equals(messageType)) {
139141
Assert.isTrue(endIndex > 0, "Expected destination pattern \"/user/{userId}/**\"");
140142
targetUser = destination.substring(startIndex, endIndex);
141143
targetDestination = destination.substring(endIndex);
142-
144+
sessionIds = this.userSessionRegistry.getSessionIds(targetUser);
143145
}
144146
else {
145147
if (logger.isTraceEnabled()) {
@@ -148,7 +150,7 @@ else if (SimpMessageType.MESSAGE.equals(messageType)) {
148150
return null;
149151
}
150152

151-
return new UserDestinationInfo(targetUser, targetDestination);
153+
return new UserDestinationInfo(targetUser, targetDestination, sessionIds);
152154
}
153155

154156
protected boolean checkDestination(String destination, String requiredPrefix) {
@@ -165,25 +167,36 @@ protected boolean checkDestination(String destination, String requiredPrefix) {
165167
return true;
166168
}
167169

170+
protected String getTargetDestination(String origDestination, String targetDestination, String sessionId, String user) {
171+
return targetDestination + "-user" + sessionId;
172+
}
173+
168174

169175
private static class UserDestinationInfo {
170176

171177
private final String user;
172178

173179
private final String destination;
174180

175-
private UserDestinationInfo(String user, String destination) {
181+
private final Set<String> sessionIds;
182+
183+
private UserDestinationInfo(String user, String destination, Set<String> sessionIds) {
176184
this.user = user;
177185
this.destination = destination;
186+
this.sessionIds = sessionIds;
178187
}
179188

180-
private String getUser() {
189+
public String getUser() {
181190
return this.user;
182191
}
183192

184-
private String getDestination() {
193+
public String getDestination() {
185194
return this.destination;
186195
}
196+
197+
public Set<String> getSessionIds() {
198+
return this.sessionIds;
199+
}
187200
}
188201

189202
}

‎spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,9 +22,6 @@
2222
import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
2323
import org.springframework.messaging.simp.SimpMessageType;
2424
import org.springframework.messaging.simp.TestPrincipal;
25-
import org.springframework.messaging.simp.user.DefaultUserDestinationResolver;
26-
import org.springframework.messaging.simp.user.DefaultUserSessionRegistry;
27-
import org.springframework.messaging.simp.user.UserSessionRegistry;
2825
import org.springframework.messaging.support.MessageBuilder;
2926

3027
import java.util.Set;
@@ -36,6 +33,8 @@
3633
*/
3734
public class DefaultUserDestinationResolverTests {
3835

36+
public static final String SESSION_ID = "123";
37+
3938
private DefaultUserDestinationResolver resolver;
4039

4140
private UserSessionRegistry registry;
@@ -44,14 +43,30 @@ public class DefaultUserDestinationResolverTests {
4443
@Before
4544
public void setup() {
4645
this.registry = new DefaultUserSessionRegistry();
46+
this.registry.registerSessionId("joe", SESSION_ID);
47+
4748
this.resolver = new DefaultUserDestinationResolver(this.registry);
4849
}
4950

5051

5152
@Test
5253
public void handleSubscribe() {
53-
Message<?> message = createMessage(SimpMessageType.SUBSCRIBE, "joe", "/user/queue/foo");
54-
this.registry.registerSessionId("joe", "123");
54+
Message<?> message = createMessage(SimpMessageType.SUBSCRIBE, "joe", SESSION_ID, "/user/queue/foo");
55+
Set<String> actual = this.resolver.resolveDestination(message);
56+
57+
assertEquals(1, actual.size());
58+
assertEquals("/queue/foo-user123", actual.iterator().next());
59+
}
60+
61+
// SPR-11325
62+
63+
@Test
64+
public void handleSubscribeOneUserMultipleSessions() {
65+
66+
this.registry.registerSessionId("joe", "456");
67+
this.registry.registerSessionId("joe", "789");
68+
69+
Message<?> message = createMessage(SimpMessageType.SUBSCRIBE, "joe", SESSION_ID, "/user/queue/foo");
5570
Set<String> actual = this.resolver.resolveDestination(message);
5671

5772
assertEquals(1, actual.size());
@@ -60,8 +75,7 @@ public void handleSubscribe() {
6075

6176
@Test
6277
public void handleUnsubscribe() {
63-
Message<?> message = createMessage(SimpMessageType.UNSUBSCRIBE, "joe", "/user/queue/foo");
64-
this.registry.registerSessionId("joe", "123");
78+
Message<?> message = createMessage(SimpMessageType.UNSUBSCRIBE, "joe", SESSION_ID, "/user/queue/foo");
6579
Set<String> actual = this.resolver.resolveDestination(message);
6680

6781
assertEquals(1, actual.size());
@@ -70,8 +84,7 @@ public void handleUnsubscribe() {
7084

7185
@Test
7286
public void handleMessage() {
73-
Message<?> message = createMessage(SimpMessageType.MESSAGE, "joe", "/user/joe/queue/foo");
74-
this.registry.registerSessionId("joe", "123");
87+
Message<?> message = createMessage(SimpMessageType.MESSAGE, "joe", SESSION_ID, "/user/joe/queue/foo");
7588
Set<String> actual = this.resolver.resolveDestination(message);
7689

7790
assertEquals(1, actual.size());
@@ -83,40 +96,43 @@ public void handleMessage() {
8396
public void ignoreMessage() {
8497

8598
// no destination
86-
Message<?> message = createMessage(SimpMessageType.MESSAGE, "joe", null);
99+
Message<?> message = createMessage(SimpMessageType.MESSAGE, "joe", SESSION_ID, null);
87100
Set<String> actual = this.resolver.resolveDestination(message);
88101
assertEquals(0, actual.size());
89102

90103
// not a user destination
91-
message = createMessage(SimpMessageType.MESSAGE, "joe", "/queue/foo");
104+
message = createMessage(SimpMessageType.MESSAGE, "joe", SESSION_ID, "/queue/foo");
92105
actual = this.resolver.resolveDestination(message);
93106
assertEquals(0, actual.size());
94107

95108
// subscribe + no user
96-
message = createMessage(SimpMessageType.SUBSCRIBE, null, "/user/queue/foo");
109+
message = createMessage(SimpMessageType.SUBSCRIBE, null, SESSION_ID, "/user/queue/foo");
97110
actual = this.resolver.resolveDestination(message);
98111
assertEquals(0, actual.size());
99112

100113
// subscribe + not a user destination
101-
message = createMessage(SimpMessageType.SUBSCRIBE, "joe", "/queue/foo");
114+
message = createMessage(SimpMessageType.SUBSCRIBE, "joe", SESSION_ID, "/queue/foo");
102115
actual = this.resolver.resolveDestination(message);
103116
assertEquals(0, actual.size());
104117

105118
// no match on message type
106-
message = createMessage(SimpMessageType.CONNECT, "joe", "user/joe/queue/foo");
119+
message = createMessage(SimpMessageType.CONNECT, "joe", SESSION_ID, "user/joe/queue/foo");
107120
actual = this.resolver.resolveDestination(message);
108121
assertEquals(0, actual.size());
109122
}
110123

111124

112-
private Message<?> createMessage(SimpMessageType messageType, String user, String destination) {
125+
private Message<?> createMessage(SimpMessageType messageType, String user, String sessionId, String destination) {
113126
SimpMessageHeaderAccessor headers = SimpMessageHeaderAccessor.create(messageType);
114127
if (destination != null) {
115128
headers.setDestination(destination);
116129
}
117130
if (user != null) {
118131
headers.setUser(new TestPrincipal(user));
119132
}
133+
if (sessionId != null) {
134+
headers.setSessionId(sessionId);
135+
}
120136
return MessageBuilder.withPayload(new byte[0]).setHeaders(headers).build();
121137
}
122138

‎spring-messaging/src/test/java/org/springframework/messaging/simp/user/UserDestinationMessageHandlerTests.java‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,10 +28,6 @@
2828
import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
2929
import org.springframework.messaging.simp.SimpMessageType;
3030
import org.springframework.messaging.simp.TestPrincipal;
31-
import org.springframework.messaging.simp.user.DefaultUserDestinationResolver;
32-
import org.springframework.messaging.simp.user.DefaultUserSessionRegistry;
33-
import org.springframework.messaging.simp.user.UserDestinationMessageHandler;
34-
import org.springframework.messaging.simp.user.UserSessionRegistry;
3531
import org.springframework.messaging.support.MessageBuilder;
3632

3733
import static org.junit.Assert.assertEquals;
@@ -42,6 +38,7 @@
4238
*/
4339
public class UserDestinationMessageHandlerTests {
4440

41+
public static final String SESSION_ID = "123";
4542
private UserDestinationMessageHandler messageHandler;
4643

4744

@@ -63,9 +60,8 @@ public void setup() {
6360

6461
@Test
6562
public void handleSubscribe() {
66-
this.registry.registerSessionId("joe", "123");
6763
when(this.brokerChannel.send(Mockito.any(Message.class))).thenReturn(true);
68-
this.messageHandler.handleMessage(createMessage(SimpMessageType.SUBSCRIBE, "joe", "/user/queue/foo"));
64+
this.messageHandler.handleMessage(createMessage(SimpMessageType.SUBSCRIBE, "joe", SESSION_ID, "/user/queue/foo"));
6965

7066
ArgumentCaptor<Message> captor = ArgumentCaptor.forClass(Message.class);
7167
Mockito.verify(this.brokerChannel).send(captor.capture());
@@ -76,9 +72,8 @@ public void handleSubscribe() {
7672

7773
@Test
7874
public void handleUnsubscribe() {
79-
this.registry.registerSessionId("joe", "123");
8075
when(this.brokerChannel.send(Mockito.any(Message.class))).thenReturn(true);
81-
this.messageHandler.handleMessage(createMessage(SimpMessageType.UNSUBSCRIBE, "joe", "/user/queue/foo"));
76+
this.messageHandler.handleMessage(createMessage(SimpMessageType.UNSUBSCRIBE, "joe", "123", "/user/queue/foo"));
8277

8378
ArgumentCaptor<Message> captor = ArgumentCaptor.forClass(Message.class);
8479
Mockito.verify(this.brokerChannel).send(captor.capture());
@@ -91,7 +86,7 @@ public void handleUnsubscribe() {
9186
public void handleMessage() {
9287
this.registry.registerSessionId("joe", "123");
9388
when(this.brokerChannel.send(Mockito.any(Message.class))).thenReturn(true);
94-
this.messageHandler.handleMessage(createMessage(SimpMessageType.MESSAGE, "joe", "/user/joe/queue/foo"));
89+
this.messageHandler.handleMessage(createMessage(SimpMessageType.MESSAGE, "joe", "123", "/user/joe/queue/foo"));
9590

9691
ArgumentCaptor<Message> captor = ArgumentCaptor.forClass(Message.class);
9792
Mockito.verify(this.brokerChannel).send(captor.capture());
@@ -105,35 +100,38 @@ public void handleMessage() {
105100
public void ignoreMessage() {
106101

107102
// no destination
108-
this.messageHandler.handleMessage(createMessage(SimpMessageType.MESSAGE, "joe", null));
103+
this.messageHandler.handleMessage(createMessage(SimpMessageType.MESSAGE, "joe", "123", null));
109104
Mockito.verifyZeroInteractions(this.brokerChannel);
110105

111106
// not a user destination
112-
this.messageHandler.handleMessage(createMessage(SimpMessageType.MESSAGE, "joe", "/queue/foo"));
107+
this.messageHandler.handleMessage(createMessage(SimpMessageType.MESSAGE, "joe", "123", "/queue/foo"));
113108
Mockito.verifyZeroInteractions(this.brokerChannel);
114109

115110
// subscribe + no user
116-
this.messageHandler.handleMessage(createMessage(SimpMessageType.SUBSCRIBE, null, "/user/queue/foo"));
111+
this.messageHandler.handleMessage(createMessage(SimpMessageType.SUBSCRIBE, null, "123", "/user/queue/foo"));
117112
Mockito.verifyZeroInteractions(this.brokerChannel);
118113

119114
// subscribe + not a user destination
120-
this.messageHandler.handleMessage(createMessage(SimpMessageType.SUBSCRIBE, "joe", "/queue/foo"));
115+
this.messageHandler.handleMessage(createMessage(SimpMessageType.SUBSCRIBE, "joe", "123", "/queue/foo"));
121116
Mockito.verifyZeroInteractions(this.brokerChannel);
122117

123118
// no match on message type
124-
this.messageHandler.handleMessage(createMessage(SimpMessageType.CONNECT, "joe", "user/joe/queue/foo"));
119+
this.messageHandler.handleMessage(createMessage(SimpMessageType.CONNECT, "joe", "123", "user/joe/queue/foo"));
125120
Mockito.verifyZeroInteractions(this.brokerChannel);
126121
}
127122

128123

129-
private Message<?> createMessage(SimpMessageType messageType, String user, String destination) {
124+
private Message<?> createMessage(SimpMessageType messageType, String user, String sessionId, String destination) {
130125
SimpMessageHeaderAccessor headers = SimpMessageHeaderAccessor.create(messageType);
131126
if (destination != null) {
132127
headers.setDestination(destination);
133128
}
134129
if (user != null) {
135130
headers.setUser(new TestPrincipal(user));
136131
}
132+
if (sessionId != null) {
133+
headers.setSessionId(sessionId);
134+
}
137135
return MessageBuilder.withPayload(new byte[0]).setHeaders(headers).build();
138136
}
139137

0 commit comments

Comments
 (0)