Skip to content

Commit ab5d60d

Browse files
rstoyanchevrstoyanchev
committed
Improve APIs for WebSocket and SockJS messages
1 parent 20a9df7 commit ab5d60d

21 files changed

+552
-225
lines changed

‎spring-websocket/src/main/java/org/springframework/sockjs/SockJsHandler.java

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,36 @@
1616

1717
package org.springframework.sockjs;
1818

19+
import org.springframework.websocket.CloseStatus;
20+
1921

2022

2123
/**
24+
* A handler for SockJS messages.
2225
*
2326
* @author Rossen Stoyanchev
2427
* @since 4.0
2528
*/
2629
public interface SockJsHandler {
2730

28-
void newSession(SockJsSession session) throws Exception;
29-
30-
void handleMessage(SockJsSession session, String message) throws Exception;
31-
32-
void handleException(SockJsSession session, Throwable exception);
33-
34-
void sessionClosed(SockJsSession session);
31+
/**
32+
* A new connection was opened and is ready for use.
33+
*/
34+
void afterConnectionEstablished(SockJsSession session) throws Exception;
35+
36+
/**
37+
* Handle an incoming message.
38+
*/
39+
void handleMessage(String message, SockJsSession session) throws Exception;
40+
41+
/**
42+
* TODO
43+
*/
44+
void handleError(Throwable exception, SockJsSession session);
45+
46+
/**
47+
* A connection has been closed.
48+
*/
49+
void afterConnectionClosed(CloseStatus status, SockJsSession session);
3550

3651
}

‎spring-websocket/src/main/java/org/springframework/sockjs/SockJsHandlerAdapter.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.sockjs;
1818

19+
import org.springframework.websocket.CloseStatus;
20+
1921

2022
/**
2123
*
@@ -25,19 +27,19 @@
2527
public class SockJsHandlerAdapter implements SockJsHandler {
2628

2729
@Override
28-
public void newSession(SockJsSession session) throws Exception {
30+
public void afterConnectionEstablished(SockJsSession session) throws Exception {
2931
}
3032

3133
@Override
32-
public void handleMessage(SockJsSession session, String message) throws Exception {
34+
public void handleMessage(String message, SockJsSession session) throws Exception {
3335
}
3436

3537
@Override
36-
public void handleException(SockJsSession session, Throwable exception) {
38+
public void handleError(Throwable exception, SockJsSession session) {
3739
}
3840

3941
@Override
40-
public void sessionClosed(SockJsSession session) {
42+
public void afterConnectionClosed(CloseStatus status, SockJsSession session) {
4143
}
4244

4345
}

‎spring-websocket/src/main/java/org/springframework/sockjs/SockJsSession.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,45 @@
1818

1919
import java.io.IOException;
2020

21+
import org.springframework.websocket.CloseStatus;
22+
2123

2224

2325
/**
26+
* Allows sending SockJS messages as well as closing the underlying connection.
2427
*
2528
* @author Rossen Stoyanchev
2629
* @since 4.0
2730
*/
2831
public interface SockJsSession {
2932

33+
/**
34+
* Return a unique SockJS session identifier.
35+
*/
3036
String getId();
3137

38+
/**
39+
* Return whether the connection is still open.
40+
*/
41+
boolean isOpen();
42+
43+
/**
44+
* Send a message.
45+
*/
3246
void sendMessage(String text) throws IOException;
3347

34-
void close();
48+
/**
49+
* Close the underlying connection with status 1000, i.e. equivalent to:
50+
* <pre>
51+
* session.close(CloseStatus.NORMAL);
52+
* </pre>
53+
*/
54+
void close() throws IOException;
55+
56+
/**
57+
* Close the underlying connection with the given close status.
58+
*/
59+
void close(CloseStatus status) throws IOException;
60+
3561

3662
}

‎spring-websocket/src/main/java/org/springframework/sockjs/SockJsSessionFactory.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919

2020
/**
21+
* A factory for creating a SockJS session.
2122
*
2223
* @author Rossen Stoyanchev
2324
* @since 4.0

‎spring-websocket/src/main/java/org/springframework/sockjs/SockJsSessionSupport.java

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
package org.springframework.sockjs;
1818

19+
import java.io.IOException;
20+
1921
import org.apache.commons.logging.Log;
2022
import org.apache.commons.logging.LogFactory;
2123
import org.springframework.util.Assert;
24+
import org.springframework.websocket.CloseStatus;
2225

2326

2427
/**
@@ -99,33 +102,78 @@ protected void updateLastActiveTime() {
99102
this.timeLastActive = System.currentTimeMillis();
100103
}
101104

102-
public void connectionInitialized() throws Exception {
105+
public void delegateConnectionEstablished() throws Exception {
103106
this.state = State.OPEN;
104-
this.sockJsHandler.newSession(this);
107+
this.sockJsHandler.afterConnectionEstablished(this);
105108
}
106109

107-
public void delegateMessages(String... messages) throws Exception {
110+
public void delegateMessages(String[] messages) throws Exception {
108111
for (String message : messages) {
109-
this.sockJsHandler.handleMessage(this, message);
112+
this.sockJsHandler.handleMessage(message, this);
110113
}
111114
}
112115

113-
public void delegateException(Throwable ex) {
114-
this.sockJsHandler.handleException(this, ex);
116+
public void delegateError(Throwable ex) {
117+
this.sockJsHandler.handleError(ex, this);
118+
}
119+
120+
/**
121+
* Invoked in reaction to the underlying connection being closed by the remote side
122+
* (or the WebSocket container) in order to perform cleanup and notify the
123+
* {@link SockJsHandler}. This is in contrast to {@link #close()} that pro-actively
124+
* closes the connection.
125+
*/
126+
public final void delegateConnectionClosed(CloseStatus status) {
127+
if (!isClosed()) {
128+
if (logger.isDebugEnabled()) {
129+
logger.debug(this + " was closed, " + status);
130+
}
131+
try {
132+
connectionClosedInternal(status);
133+
}
134+
finally {
135+
this.state = State.CLOSED;
136+
this.sockJsHandler.afterConnectionClosed(status, this);
137+
}
138+
}
115139
}
116140

117-
public void connectionClosed() {
118-
this.state = State.CLOSED;
119-
this.sockJsHandler.sessionClosed(this);
141+
protected void connectionClosedInternal(CloseStatus status) {
120142
}
121143

122-
public void close() {
123-
this.state = State.CLOSED;
124-
this.sockJsHandler.sessionClosed(this);
144+
/**
145+
* {@inheritDoc}
146+
* <p>Performs cleanup and notifies the {@link SockJsHandler}.
147+
*/
148+
public final void close() throws IOException {
149+
close(CloseStatus.NORMAL);
125150
}
126151

152+
/**
153+
* {@inheritDoc}
154+
* <p>Performs cleanup and notifies the {@link SockJsHandler}.
155+
*/
156+
public final void close(CloseStatus status) throws IOException {
157+
if (!isClosed()) {
158+
if (logger.isDebugEnabled()) {
159+
logger.debug("Closing " + this + ", " + status);
160+
}
161+
try {
162+
closeInternal(status);
163+
}
164+
finally {
165+
this.state = State.CLOSED;
166+
this.sockJsHandler.afterConnectionClosed(status, this);
167+
}
168+
}
169+
}
170+
171+
protected abstract void closeInternal(CloseStatus status) throws IOException;
172+
173+
174+
@Override
127175
public String toString() {
128-
return getClass().getSimpleName() + " [id=" + sessionId + "]";
176+
return "SockJS session id=" + this.sessionId;
129177
}
130178

131179

‎spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.springframework.sockjs.SockJsSession;
2727
import org.springframework.sockjs.SockJsSessionSupport;
2828
import org.springframework.util.Assert;
29+
import org.springframework.websocket.CloseStatus;
2930

3031

3132
/**
@@ -42,9 +43,9 @@ public abstract class AbstractServerSockJsSession extends SockJsSessionSupport {
4243
private ScheduledFuture<?> heartbeatTask;
4344

4445

45-
public AbstractServerSockJsSession(String sessionId, SockJsConfiguration sockJsConfig, SockJsHandler sockJsHandler) {
46+
public AbstractServerSockJsSession(String sessionId, SockJsConfiguration config, SockJsHandler sockJsHandler) {
4647
super(sessionId, sockJsHandler);
47-
this.sockJsConfig = sockJsConfig;
48+
this.sockJsConfig = config;
4849
}
4950

5051
protected SockJsConfiguration getSockJsConfig() {
@@ -60,36 +61,34 @@ public final synchronized void sendMessage(String message) throws IOException {
6061

6162

6263
@Override
63-
public void connectionClosed() {
64-
logger.debug("Session closed");
65-
super.close();
64+
public void connectionClosedInternal(CloseStatus status) {
65+
updateLastActiveTime();
6666
cancelHeartbeat();
6767
}
6868

6969
@Override
70-
public final synchronized void close() {
71-
if (!isClosed()) {
72-
logger.debug("Closing session");
73-
if (isActive()) {
74-
// deliver messages "in flight" before sending close frame
75-
try {
76-
writeFrame(SockJsFrame.closeFrameGoAway());
77-
}
78-
catch (Exception e) {
79-
// ignore
80-
}
70+
public final synchronized void closeInternal(CloseStatus status) throws IOException {
71+
if (isActive()) {
72+
// TODO: deliver messages "in flight" before sending close frame
73+
try {
74+
// bypass writeFrame
75+
writeFrameInternal(SockJsFrame.closeFrame(status.getCode(), status.getReason()));
76+
}
77+
catch (Throwable ex) {
78+
logger.warn("Failed to send SockJS close frame: " + ex.getMessage());
8179
}
82-
super.close();
83-
cancelHeartbeat();
84-
closeInternal();
8580
}
81+
updateLastActiveTime();
82+
cancelHeartbeat();
83+
disconnect(status);
8684
}
8785

88-
protected abstract void closeInternal();
86+
// TODO: close status/reason
87+
protected abstract void disconnect(CloseStatus status) throws IOException;
8988

9089
/**
9190
* For internal use within a TransportHandler and the (TransportHandler-specific)
92-
* session sub-class. The frame is written only if the connection is active.
91+
* session sub-class.
9392
*/
9493
protected void writeFrame(SockJsFrame frame) throws IOException {
9594
if (logger.isTraceEnabled()) {
@@ -103,29 +102,20 @@ protected void writeFrame(SockJsFrame frame) throws IOException {
103102
logger.warn("Client went away. Terminating connection");
104103
}
105104
else {
106-
logger.warn("Failed to send message. Terminating connection: " + ex.getMessage());
105+
logger.warn("Terminating connection due to failure to send message: " + ex.getMessage());
107106
}
108-
deactivate();
109107
close();
110108
throw ex;
111109
}
112-
catch (Throwable t) {
113-
logger.warn("Failed to send message. Terminating connection: " + t.getMessage());
114-
deactivate();
110+
catch (Throwable ex) {
111+
logger.warn("Terminating connection due to failure to send message: " + ex.getMessage());
115112
close();
116-
throw new NestedSockJsRuntimeException("Failed to write frame " + frame, t);
113+
throw new NestedSockJsRuntimeException("Failed to write frame " + frame, ex);
117114
}
118115
}
119116

120117
protected abstract void writeFrameInternal(SockJsFrame frame) throws IOException;
121118

122-
/**
123-
* Some {@link TransportHandler} types cannot detect if a client connection is closed
124-
* or lost and will eventually fail to send messages. When that happens, we need a way
125-
* to disconnect the underlying connection before calling {@link #close()}.
126-
*/
127-
protected abstract void deactivate();
128-
129119
public synchronized void sendHeartbeat() throws IOException {
130120
if (isActive()) {
131121
writeFrame(SockJsFrame.heartbeatFrame());

‎spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractSockJsService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public abstract class AbstractSockJsService
5656
private static final int ONE_YEAR = 365 * 24 * 60 * 60;
5757

5858

59-
private String name = getClass().getSimpleName() + "@" + Integer.toHexString(hashCode());
59+
private String name = getClass().getSimpleName() + "@" + ObjectUtils.getIdentityHexString(this);
6060

6161
private String clientLibraryUrl = "https://d1fxtkz8shb9d2.cloudfront.net/sockjs-0.3.4.min.js";
6262

‎spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpSendingTransportHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ protected void handleNewSession(ServerHttpRequest request, ServerHttpResponse re
9292
logger.debug("Opening " + getTransportType() + " connection");
9393
session.setFrameFormat(getFrameFormat(request));
9494
session.writeFrame(response, SockJsFrame.openFrame());
95-
session.connectionInitialized();
95+
session.delegateConnectionEstablished();
9696
}
9797

9898
protected abstract MediaType getContentType();

0 commit comments

Comments
 (0)