Skip to content

Commit 5c46efe

Browse files
artembilangaryrussell
authored andcommitted
Fix Sonar smells for websocket module
* Introduce `JavaUtils` for chaining properties setting * Fix method complexity in the `ServerWebSocketContainer` using newly introduced `JavaUtils` * Make `JavaUtils` as `final`
1 parent 5eba369 commit 5c46efe

File tree

9 files changed

+210
-114
lines changed

9 files changed

+210
-114
lines changed
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Copyright 2019 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.integration.util;
18+
19+
import java.util.function.Consumer;
20+
21+
/**
22+
* Chained utility methods to simplify some Java repetitive code. Obtain a reference to
23+
* the singleton {@link #INSTANCE} and then chain calls to the utility methods.
24+
*
25+
* @author Gary Russell
26+
* @author Artem Bilan
27+
*
28+
* @since 5.1.3
29+
*/
30+
public final class JavaUtils {
31+
32+
/**
33+
* The singleton instance of this utility class.
34+
*/
35+
public static final JavaUtils INSTANCE = new JavaUtils();
36+
37+
private JavaUtils() {
38+
super();
39+
}
40+
41+
/**
42+
* Invoke {@link Consumer#accept(Object)} with the value if the condition is true.
43+
* @param condition the condition.
44+
* @param value the value.
45+
* @param consumer the consumer.
46+
* @param <T> the value type.
47+
* @return this.
48+
*/
49+
public <T> JavaUtils acceptIfCondition(boolean condition, T value, Consumer<T> consumer) {
50+
if (condition) {
51+
consumer.accept(value);
52+
}
53+
return this;
54+
}
55+
56+
/**
57+
* Invoke {@link Consumer#accept(Object)} with the value if it is not null.
58+
* @param value the value.
59+
* @param consumer the consumer.
60+
* @param <T> the value type.
61+
* @return this.
62+
*/
63+
public <T> JavaUtils acceptIfNotNull(T value, Consumer<T> consumer) {
64+
if (value != null) {
65+
consumer.accept(value);
66+
}
67+
return this;
68+
}
69+
70+
}

spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ClientWebSocketContainer.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2017 the original author or authors.
2+
* Copyright 2014-2019 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.
@@ -46,6 +46,7 @@
4646
*
4747
* @author Artem Bilan
4848
* @author Gary Russell
49+
*
4950
* @since 4.1
5051
*/
5152
public final class ClientWebSocketContainer extends IntegrationWebSocketContainer implements SmartLifecycle {
@@ -221,7 +222,7 @@ public void startInternal() {
221222
}
222223

223224
@Override
224-
public void stopInternal() throws Exception {
225+
public void stopInternal() throws Exception { // NOSONAR honor super
225226
if (this.syncClientLifecycle) {
226227
((Lifecycle) this.client).stop();
227228
}
@@ -236,7 +237,9 @@ public void stopInternal() throws Exception {
236237

237238
@Override
238239
protected void openConnection() {
239-
logger.info("Connecting to WebSocket at " + getUri());
240+
if (logger.isInfoEnabled()) {
241+
logger.info("Connecting to WebSocket at " + getUri());
242+
}
240243
ClientWebSocketContainer.this.headers.setSecWebSocketProtocol(getSubProtocols());
241244
ListenableFuture<WebSocketSession> future =
242245
this.client.doHandshake(ClientWebSocketContainer.this.webSocketHandler,
@@ -262,10 +265,9 @@ public void onFailure(Throwable t) {
262265
}
263266

264267
@Override
265-
protected void closeConnection() throws Exception {
268+
protected void closeConnection() throws Exception { // NOSONAR
266269
if (ClientWebSocketContainer.this.clientSession != null) {
267-
ClientWebSocketContainer.this.closeSession(ClientWebSocketContainer.this.clientSession,
268-
CloseStatus.NORMAL);
270+
closeSession(ClientWebSocketContainer.this.clientSession, CloseStatus.NORMAL);
269271
}
270272
}
271273

spring-integration-websocket/src/main/java/org/springframework/integration/websocket/IntegrationWebSocketContainer.java

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import org.springframework.beans.factory.DisposableBean;
2929
import org.springframework.util.Assert;
30+
import org.springframework.util.ReflectionUtils;
3031
import org.springframework.web.socket.CloseStatus;
3132
import org.springframework.web.socket.SubProtocolCapable;
3233
import org.springframework.web.socket.WebSocketHandler;
@@ -52,25 +53,31 @@
5253
*
5354
* @author Artem Bilan
5455
* @author Gary Russell
56+
*
5557
* @since 4.1
58+
*
5659
* @see org.springframework.integration.websocket.inbound.WebSocketInboundChannelAdapter
5760
* @see org.springframework.integration.websocket.outbound.WebSocketOutboundMessageHandler
5861
*/
5962
public abstract class IntegrationWebSocketContainer implements DisposableBean {
6063

61-
protected final Log logger = LogFactory.getLog(this.getClass());
64+
public static final int DEFAULT_SEND_TIME_LIMIT = 10 * 1000;
65+
66+
public static final int DEFAULT_SEND_BUFFER_SIZE = 512 * 1024;
6267

63-
protected final WebSocketHandler webSocketHandler = new IntegrationWebSocketHandler();
68+
protected final Log logger = LogFactory.getLog(getClass()); // NOSONAR
6469

65-
protected final Map<String, WebSocketSession> sessions = new ConcurrentHashMap<String, WebSocketSession>();
70+
protected final WebSocketHandler webSocketHandler = new IntegrationWebSocketHandler(); // NOSONAR
6671

67-
private final List<String> supportedProtocols = new ArrayList<String>();
72+
protected final Map<String, WebSocketSession> sessions = new ConcurrentHashMap<>(); // NOSONAR
73+
74+
private final List<String> supportedProtocols = new ArrayList<>();
6875

6976
private volatile WebSocketListener messageListener;
7077

71-
private volatile int sendTimeLimit = 10 * 1000;
78+
private volatile int sendTimeLimit = DEFAULT_SEND_TIME_LIMIT;
7279

73-
private volatile int sendBufferSizeLimit = 512 * 1024;
80+
private volatile int sendBufferSizeLimit = DEFAULT_SEND_BUFFER_SIZE;
7481

7582
public void setSendTimeLimit(int sendTimeLimit) {
7683
this.sendTimeLimit = sendTimeLimit;
@@ -98,7 +105,7 @@ public void addSupportedProtocols(String... protocols) {
98105
}
99106

100107
public List<String> getSubProtocols() {
101-
List<String> protocols = new ArrayList<String>();
108+
List<String> protocols = new ArrayList<>();
102109
if (this.messageListener != null) {
103110
protocols.addAll(this.messageListener.getSubProtocols());
104111
}
@@ -116,14 +123,16 @@ public WebSocketSession getSession(String sessionId) {
116123
return session;
117124
}
118125

119-
public void closeSession(WebSocketSession session, CloseStatus closeStatus) throws Exception {
126+
public void closeSession(WebSocketSession session, CloseStatus closeStatus)
127+
throws Exception { // NOSONAR
128+
120129
// Session may be unresponsive so clear first
121130
session.close(closeStatus);
122131
this.webSocketHandler.afterConnectionClosed(session, closeStatus);
123132
}
124133

125134
@Override
126-
public void destroy() throws Exception {
135+
public void destroy() {
127136
try {
128137
// Notify sessions to stop flushing messages
129138
for (WebSocketSession session : this.sessions.values()) {
@@ -159,45 +168,54 @@ public List<String> getSubProtocols() {
159168
}
160169

161170
@Override
162-
public void afterConnectionEstablished(WebSocketSession sessionToDecorate) throws Exception { // NOSONAR SF ifce
163-
WebSocketSession session = new ConcurrentWebSocketSessionDecorator(sessionToDecorate,
164-
IntegrationWebSocketContainer.this.sendTimeLimit,
165-
IntegrationWebSocketContainer.this.sendBufferSizeLimit);
171+
public void afterConnectionEstablished(WebSocketSession sessionToDecorate)
172+
throws Exception { // NOSONAR
173+
174+
WebSocketSession session =
175+
new ConcurrentWebSocketSessionDecorator(sessionToDecorate,
176+
IntegrationWebSocketContainer.this.sendTimeLimit,
177+
IntegrationWebSocketContainer.this.sendBufferSizeLimit);
166178

167179
IntegrationWebSocketContainer.this.sessions.put(session.getId(), session);
168180
if (IntegrationWebSocketContainer.this.logger.isDebugEnabled()) {
169-
IntegrationWebSocketContainer.this.logger.debug("Started WebSocket session = " + session.getId() + ", number of sessions = "
170-
+ IntegrationWebSocketContainer.this.sessions.size());
181+
IntegrationWebSocketContainer.this.logger.debug("Started WebSocket session = " +
182+
session.getId() + ", number of sessions = " +
183+
IntegrationWebSocketContainer.this.sessions.size());
171184
}
172185
if (IntegrationWebSocketContainer.this.messageListener != null) {
173186
IntegrationWebSocketContainer.this.messageListener.afterSessionStarted(session);
174187
}
175188
}
176189

177190
@Override
178-
public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception {
191+
public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus)
192+
throws Exception { // NOSONAR
193+
179194
WebSocketSession removed = IntegrationWebSocketContainer.this.sessions.remove(session.getId());
180-
if (removed != null) {
181-
if (IntegrationWebSocketContainer.this.messageListener != null) {
182-
IntegrationWebSocketContainer.this.messageListener.afterSessionEnded(session, closeStatus);
183-
}
195+
if (removed != null && IntegrationWebSocketContainer.this.messageListener != null) {
196+
IntegrationWebSocketContainer.this.messageListener.afterSessionEnded(session, closeStatus);
184197
}
185198
}
186199

187200
@Override
188-
public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception {
201+
public void handleTransportError(WebSocketSession session, Throwable exception)
202+
throws Exception { // NOSONAR
203+
189204
IntegrationWebSocketContainer.this.sessions.remove(session.getId());
190-
throw new Exception(exception);
205+
ReflectionUtils.rethrowException(exception);
191206
}
192207

193208
@Override
194-
public void handleMessage(WebSocketSession session, WebSocketMessage<?> message) throws Exception {
209+
public void handleMessage(WebSocketSession session, WebSocketMessage<?> message)
210+
throws Exception { // NOSONAR
211+
195212
if (IntegrationWebSocketContainer.this.messageListener != null) {
196213
IntegrationWebSocketContainer.this.messageListener.onMessage(session, message);
197214
}
198215
else if (IntegrationWebSocketContainer.this.logger.isInfoEnabled()) {
199-
IntegrationWebSocketContainer.this.logger.info("This 'WebSocketHandlerContainer' isn't configured with 'WebSocketMessageListener'."
200-
+ " Received messages are ignored. Current message is: " + message);
216+
IntegrationWebSocketContainer.this.logger.info("This 'WebSocketHandlerContainer' isn't " +
217+
"configured with 'WebSocketMessageListener'." +
218+
" Received messages are ignored. Current message is: " + message);
201219
}
202220
}
203221

spring-integration-websocket/src/main/java/org/springframework/integration/websocket/ServerWebSocketContainer.java

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2016 the original author or authors.
2+
* Copyright 2014-2019 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.
@@ -20,6 +20,7 @@
2020

2121
import org.springframework.context.Lifecycle;
2222
import org.springframework.context.SmartLifecycle;
23+
import org.springframework.integration.util.JavaUtils;
2324
import org.springframework.scheduling.TaskScheduler;
2425
import org.springframework.util.Assert;
2526
import org.springframework.util.ObjectUtils;
@@ -46,6 +47,7 @@
4647
*
4748
* @author Artem Bilan
4849
* @author Gary Russell
50+
*
4951
* @since 4.1
5052
*/
5153
public class ServerWebSocketContainer extends IntegrationWebSocketContainer
@@ -142,47 +144,38 @@ public void registerWebSocketHandlers(WebSocketHandlerRegistry registry) {
142144
.addInterceptors(this.interceptors)
143145
.setAllowedOrigins(this.origins);
144146

147+
configureSockJsOptionsIfAny(registration);
148+
}
149+
150+
private void configureSockJsOptionsIfAny(WebSocketHandlerRegistration registration) {
145151
if (this.sockJsServiceOptions != null) {
146152
SockJsServiceRegistration sockJsServiceRegistration = registration.withSockJS();
147-
if (this.sockJsServiceOptions.webSocketEnabled != null) {
148-
sockJsServiceRegistration.setWebSocketEnabled(this.sockJsServiceOptions.webSocketEnabled);
149-
}
150-
if (this.sockJsServiceOptions.clientLibraryUrl != null) {
151-
sockJsServiceRegistration.setClientLibraryUrl(this.sockJsServiceOptions.clientLibraryUrl);
152-
}
153-
if (this.sockJsServiceOptions.disconnectDelay != null) {
154-
sockJsServiceRegistration.setDisconnectDelay(this.sockJsServiceOptions.disconnectDelay);
155-
}
156-
if (this.sockJsServiceOptions.heartbeatTime != null) {
157-
sockJsServiceRegistration.setHeartbeatTime(this.sockJsServiceOptions.heartbeatTime);
158-
}
159-
if (this.sockJsServiceOptions.httpMessageCacheSize != null) {
160-
sockJsServiceRegistration.setHttpMessageCacheSize(this.sockJsServiceOptions.httpMessageCacheSize);
161-
}
162-
if (this.sockJsServiceOptions.heartbeatTime != null) {
163-
sockJsServiceRegistration.setHeartbeatTime(this.sockJsServiceOptions.heartbeatTime);
164-
}
165-
if (this.sockJsServiceOptions.sessionCookieNeeded != null) {
166-
sockJsServiceRegistration.setSessionCookieNeeded(this.sockJsServiceOptions.sessionCookieNeeded);
167-
}
168-
if (this.sockJsServiceOptions.streamBytesLimit != null) {
169-
sockJsServiceRegistration.setStreamBytesLimit(this.sockJsServiceOptions.streamBytesLimit);
170-
}
171-
if (this.sockJsServiceOptions.transportHandlers != null) {
172-
sockJsServiceRegistration.setTransportHandlers(this.sockJsServiceOptions.transportHandlers);
173-
}
174-
if (this.sockJsServiceOptions.taskScheduler != null) {
175-
sockJsServiceRegistration.setTaskScheduler(this.sockJsServiceOptions.taskScheduler);
176-
}
177-
if (this.sockJsServiceOptions.messageCodec != null) {
178-
sockJsServiceRegistration.setMessageCodec(this.sockJsServiceOptions.messageCodec);
179-
}
180-
if (this.sockJsServiceOptions.suppressCors != null) {
181-
sockJsServiceRegistration.setSupressCors(this.sockJsServiceOptions.suppressCors);
182-
}
183-
153+
JavaUtils.INSTANCE
154+
.acceptIfNotNull(this.sockJsServiceOptions.webSocketEnabled,
155+
sockJsServiceRegistration::setWebSocketEnabled)
156+
.acceptIfNotNull(this.sockJsServiceOptions.clientLibraryUrl,
157+
sockJsServiceRegistration::setClientLibraryUrl)
158+
.acceptIfNotNull(this.sockJsServiceOptions.disconnectDelay,
159+
sockJsServiceRegistration::setDisconnectDelay)
160+
.acceptIfNotNull(this.sockJsServiceOptions.heartbeatTime,
161+
sockJsServiceRegistration::setHeartbeatTime)
162+
.acceptIfNotNull(this.sockJsServiceOptions.httpMessageCacheSize,
163+
sockJsServiceRegistration::setHttpMessageCacheSize)
164+
.acceptIfNotNull(this.sockJsServiceOptions.heartbeatTime,
165+
sockJsServiceRegistration::setHeartbeatTime)
166+
.acceptIfNotNull(this.sockJsServiceOptions.sessionCookieNeeded,
167+
sockJsServiceRegistration::setSessionCookieNeeded)
168+
.acceptIfNotNull(this.sockJsServiceOptions.streamBytesLimit,
169+
sockJsServiceRegistration::setStreamBytesLimit)
170+
.acceptIfNotNull(this.sockJsServiceOptions.transportHandlers,
171+
sockJsServiceRegistration::setTransportHandlers)
172+
.acceptIfNotNull(this.sockJsServiceOptions.taskScheduler,
173+
sockJsServiceRegistration::setTaskScheduler)
174+
.acceptIfNotNull(this.sockJsServiceOptions.messageCodec,
175+
sockJsServiceRegistration::setMessageCodec)
176+
.acceptIfNotNull(this.sockJsServiceOptions.suppressCors,
177+
sockJsServiceRegistration::setSupressCors);
184178
}
185-
186179
}
187180

188181
public void setAutoStartup(boolean autoStartup) {

0 commit comments

Comments
 (0)