Skip to content

Commit 8ce8bf9

Browse files
artembilangaryrussell
artembilan
authored and
garyrussell
committed
Fix code smell in websocket and webflux modules (#2669)
* * Fix code smell in websocket and webflux modules * * More fixes * * Fix NPE in the `IntegrationHandlerResultHandler`
1 parent e8bd31c commit 8ce8bf9

File tree

9 files changed

+138
-91
lines changed

9 files changed

+138
-91
lines changed

‎spring-integration-core/src/main/java/org/springframework/integration/transformer/PayloadTypeConvertingTransformer.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ protected Converter<T, U> getConverter() {
5353
}
5454

5555
@Override
56-
protected void onInit() throws Exception {
56+
protected void onInit() throws Exception { // NOSONAR thrown by super class
5757
super.onInit();
5858
Assert.notNull(this.converter, () -> getClass().getName() + " requires a Converter<Object, Object>");
5959
}

‎spring-integration-scripting/src/main/java/org/springframework/integration/scripting/dsl/DslScriptExecutingMessageProcessor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public void afterPropertiesSet() {
105105
String filename = this.script.getFilename();
106106
int index =
107107
filename != null
108-
? filename.lastIndexOf(".") + 1
108+
? filename.lastIndexOf('.') + 1
109109
: -1;
110110
if (index < 1) {
111111
throw new BeanCreationException(

‎spring-integration-webflux/src/main/java/org/springframework/integration/webflux/inbound/IntegrationHandlerResultHandler.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017 the original author or authors.
2+
* Copyright 2017-2018 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.
@@ -47,7 +47,8 @@ public boolean supports(HandlerResult result) {
4747
@Override
4848
@SuppressWarnings("unchecked")
4949
public Mono<Void> handleResult(ServerWebExchange exchange, HandlerResult result) {
50-
return (Mono<Void>) result.getReturnValue();
50+
Object returnValue = result.getReturnValue();
51+
return returnValue == null ? Mono.empty() : (Mono<Void>) returnValue;
5152
}
5253

5354
@Override

‎spring-integration-webflux/src/main/java/org/springframework/integration/webflux/inbound/WebFluxInboundEndpoint.java

+53-44
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
8080

8181
private static final MediaType MEDIA_TYPE_APPLICATION_ALL = new MediaType("application");
8282

83+
private static final String UNCHECKED = "unchecked";
84+
8385
private static final List<HttpMethod> SAFE_METHODS = Arrays.asList(HttpMethod.GET, HttpMethod.HEAD);
8486

8587
private ServerCodecConfigurer codecConfigurer = ServerCodecConfigurer.create();
@@ -130,11 +132,6 @@ public String getComponentType() {
130132
return super.getComponentType().replaceFirst("http", "webflux");
131133
}
132134

133-
@Override
134-
protected void onInit() throws Exception {
135-
super.onInit();
136-
}
137-
138135
@Override
139136
public Mono<Void> handle(ServerWebExchange exchange) {
140137
return Mono.defer(() -> {
@@ -147,7 +144,6 @@ public Mono<Void> handle(ServerWebExchange exchange) {
147144
});
148145
}
149146

150-
@SuppressWarnings("unchecked")
151147
private Mono<Void> doHandle(ServerWebExchange exchange) {
152148
return extractRequestBody(exchange)
153149
.doOnSubscribe(s -> this.activeCount.incrementAndGet())
@@ -170,7 +166,7 @@ private Mono<Void> doHandle(ServerWebExchange exchange) {
170166

171167
}
172168

173-
@SuppressWarnings("unchecked")
169+
@SuppressWarnings(UNCHECKED)
174170
private <T> Mono<T> extractRequestBody(ServerWebExchange exchange) {
175171
ServerHttpRequest request = exchange.getRequest();
176172
ServerHttpResponse response = exchange.getResponse();
@@ -201,7 +197,8 @@ else if (MediaType.MULTIPART_FORM_DATA.isCompatibleWith(contentType)) {
201197

202198
Class<?> resolvedType = bodyType.resolve();
203199

204-
ReactiveAdapter adapter = (resolvedType != null ? this.adapterRegistry.getAdapter(resolvedType) : null);
200+
ReactiveAdapter adapter = (resolvedType != null ? this.adapterRegistry.getAdapter(resolvedType) :
201+
null);
205202
ResolvableType elementType = (adapter != null ? bodyType.getGeneric() : bodyType);
206203

207204
HttpMessageReader<?> httpMessageReader = this.codecConfigurer
@@ -237,40 +234,14 @@ else if (MediaType.MULTIPART_FORM_DATA.isCompatibleWith(contentType)) {
237234
}
238235
}
239236

240-
@SuppressWarnings("unchecked")
237+
@SuppressWarnings(UNCHECKED)
241238
private Mono<Tuple2<Message<Object>, RequestEntity<?>>> buildMessage(RequestEntity<?> httpEntity,
242239
ServerWebExchange exchange) {
243240

244241
ServerHttpRequest request = exchange.getRequest();
245-
HttpHeaders requestHeaders = request.getHeaders();
246-
Map<String, Object> exchangeAttributes = exchange.getAttributes();
247-
248-
StandardEvaluationContext evaluationContext = createEvaluationContext();
249-
250-
evaluationContext.setVariable("requestAttributes", exchangeAttributes);
251242
MultiValueMap<String, String> requestParams = request.getQueryParams();
252-
evaluationContext.setVariable("requestParams", requestParams);
253-
evaluationContext.setVariable("requestHeaders", requestHeaders);
254-
if (!CollectionUtils.isEmpty(request.getCookies())) {
255-
evaluationContext.setVariable("cookies", request.getCookies());
256-
}
257-
258-
Map<String, String> pathVariables =
259-
(Map<String, String>) exchangeAttributes.get(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
260-
261-
if (!CollectionUtils.isEmpty(pathVariables)) {
262-
evaluationContext.setVariable("pathVariables", pathVariables);
263-
}
264-
265-
Map<String, MultiValueMap<String, String>> matrixVariables =
266-
(Map<String, MultiValueMap<String, String>>) exchangeAttributes
267-
.get(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE);
268-
269-
if (!CollectionUtils.isEmpty(matrixVariables)) {
270-
evaluationContext.setVariable("matrixVariables", matrixVariables);
271-
}
272243

273-
evaluationContext.setRootObject(httpEntity);
244+
StandardEvaluationContext evaluationContext = buildEvaluationContext(httpEntity, exchange);
274245
Object payload;
275246
if (getPayloadExpression() != null) {
276247
payload = getPayloadExpression().getValue(evaluationContext);
@@ -310,10 +281,13 @@ private Mono<Tuple2<Message<Object>, RequestEntity<?>>> buildMessage(RequestEnti
310281
.copyHeaders(headers);
311282
}
312283

313-
messageBuilder
314-
.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_URL, request.getURI().toString())
315-
.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_METHOD,
316-
request.getMethod().toString());
284+
messageBuilder.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_URL,
285+
request.getURI().toString());
286+
HttpMethod httpMethod = request.getMethod();
287+
if (httpMethod != null) {
288+
messageBuilder.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_METHOD,
289+
httpMethod.toString());
290+
}
317291

318292
return exchange.getPrincipal()
319293
.map(principal ->
@@ -324,6 +298,41 @@ private Mono<Tuple2<Message<Object>, RequestEntity<?>>> buildMessage(RequestEnti
324298
.zipWith(Mono.just(httpEntity));
325299
}
326300

301+
@SuppressWarnings(UNCHECKED)
302+
private StandardEvaluationContext buildEvaluationContext(RequestEntity<?> httpEntity, ServerWebExchange exchange) {
303+
ServerHttpRequest request = exchange.getRequest();
304+
HttpHeaders requestHeaders = request.getHeaders();
305+
MultiValueMap<String, String> requestParams = request.getQueryParams();
306+
Map<String, Object> exchangeAttributes = exchange.getAttributes();
307+
308+
StandardEvaluationContext evaluationContext = createEvaluationContext();
309+
310+
evaluationContext.setVariable("requestAttributes", exchangeAttributes);
311+
evaluationContext.setVariable("requestParams", requestParams);
312+
evaluationContext.setVariable("requestHeaders", requestHeaders);
313+
if (!CollectionUtils.isEmpty(request.getCookies())) {
314+
evaluationContext.setVariable("cookies", request.getCookies());
315+
}
316+
317+
Map<String, String> pathVariables =
318+
(Map<String, String>) exchangeAttributes.get(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
319+
320+
if (!CollectionUtils.isEmpty(pathVariables)) {
321+
evaluationContext.setVariable("pathVariables", pathVariables);
322+
}
323+
324+
Map<String, MultiValueMap<String, String>> matrixVariables =
325+
(Map<String, MultiValueMap<String, String>>) exchangeAttributes
326+
.get(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE);
327+
328+
if (!CollectionUtils.isEmpty(matrixVariables)) {
329+
evaluationContext.setVariable("matrixVariables", matrixVariables);
330+
}
331+
332+
evaluationContext.setRootObject(httpEntity);
333+
return evaluationContext;
334+
}
335+
327336
private Mono<Void> populateResponse(ServerWebExchange exchange, Message<?> replyMessage) {
328337
ServerHttpResponse response = exchange.getResponse();
329338
getHeaderMapper().fromHeaders(replyMessage.getHeaders(), response.getHeaders());
@@ -379,7 +388,7 @@ private Mono<Void> populateResponse(ServerWebExchange exchange, Message<?> reply
379388
}
380389
}
381390

382-
@SuppressWarnings("unchecked")
391+
@SuppressWarnings(UNCHECKED)
383392
private Mono<Void> writeResponseBody(ServerWebExchange exchange, Object body) {
384393
ResolvableType bodyType = ResolvableType.forInstance(body);
385394
ReactiveAdapter adapter = this.adapterRegistry.getAdapter(bodyType.resolve(), body);
@@ -473,7 +482,7 @@ private List<MediaType> getAcceptableTypes(ServerWebExchange exchange) {
473482
return (mediaTypes.isEmpty() ? Collections.singletonList(MediaType.ALL) : mediaTypes);
474483
}
475484

476-
@SuppressWarnings("unchecked")
485+
@SuppressWarnings(UNCHECKED)
477486
private List<MediaType> getProducibleTypes(ServerWebExchange exchange,
478487
Supplier<List<MediaType>> producibleTypesSupplier) {
479488

@@ -482,9 +491,9 @@ private List<MediaType> getProducibleTypes(ServerWebExchange exchange,
482491
}
483492

484493
private MediaType selectMoreSpecificMediaType(MediaType acceptable, MediaType producible) {
485-
producible = producible.copyQualityValue(acceptable);
494+
MediaType producibleToUse = producible.copyQualityValue(acceptable);
486495
Comparator<MediaType> comparator = MediaType.SPECIFICITY_COMPARATOR;
487-
return (comparator.compare(acceptable, producible) <= 0 ? acceptable : producible);
496+
return (comparator.compare(acceptable, producibleToUse) <= 0 ? acceptable : producibleToUse);
488497
}
489498

490499

‎spring-integration-webflux/src/main/java/org/springframework/integration/webflux/inbound/WebFluxIntegrationRequestMappingHandlerMapping.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,11 @@ protected boolean isHandler(Class<?> beanType) {
119119
@Override
120120
protected void detectHandlerMethods(Object handler) {
121121
if (handler instanceof String) {
122-
handler = getApplicationContext().getBean((String) handler);
122+
handler = getApplicationContext().getBean((String) handler); // NOSONAR never null
123123
}
124124
RequestMappingInfo mapping = getMappingForEndpoint((WebFluxInboundEndpoint) handler);
125125
if (mapping != null) {
126-
registerMapping(mapping, handler, HANDLER_METHOD);
126+
registerMapping(mapping, handler, HANDLER_METHOD); // NOSONAR never null
127127
}
128128
}
129129

‎spring-integration-webflux/src/main/java/org/springframework/integration/webflux/outbound/WebFluxRequestExecutingMessageHandler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ protected Object exchange(Supplier<URI> uriSupplier, HttpMethod httpMethod, Http
159159
.headers(headers -> headers.putAll(httpRequest.getHeaders()));
160160

161161
if (httpRequest.hasBody()) {
162-
requestSpec.body(BodyInserters.fromObject(httpRequest.getBody()));
162+
requestSpec.body(BodyInserters.fromObject(httpRequest.getBody())); // NOSONAR protected with hasBody()
163163
}
164164

165165
Mono<ClientResponse> responseMono =

‎spring-integration-websocket/src/main/java/org/springframework/integration/websocket/config/WebSocketIntegrationConfigurationInitializer.java

+14-11
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-2018 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.
@@ -16,12 +16,11 @@
1616

1717
package org.springframework.integration.websocket.config;
1818

19-
import java.util.Collection;
20-
2119
import org.apache.commons.logging.Log;
2220
import org.apache.commons.logging.LogFactory;
2321

2422
import org.springframework.beans.BeansException;
23+
import org.springframework.beans.factory.BeanFactory;
2524
import org.springframework.beans.factory.ListableBeanFactory;
2625
import org.springframework.beans.factory.config.AbstractFactoryBean;
2726
import org.springframework.beans.factory.config.BeanDefinition;
@@ -50,10 +49,12 @@
5049
*/
5150
public class WebSocketIntegrationConfigurationInitializer implements IntegrationConfigurationInitializer {
5251

53-
private static final Log logger = LogFactory.getLog(WebSocketIntegrationConfigurationInitializer.class);
52+
private static final Log logger = // NOSONAR lower case
53+
LogFactory.getLog(WebSocketIntegrationConfigurationInitializer.class);
5454

55-
private static final boolean servletPresent = ClassUtils.isPresent("javax.servlet.Servlet",
56-
WebSocketIntegrationConfigurationInitializer.class.getClassLoader());
55+
private static final boolean servletPresent = // NOSONAR lower case
56+
ClassUtils.isPresent("javax.servlet.Servlet",
57+
WebSocketIntegrationConfigurationInitializer.class.getClassLoader());
5758

5859
private static final String WEB_SOCKET_HANDLER_MAPPING_BEAN_NAME = "integrationWebSocketHandlerMapping";
5960

@@ -130,11 +131,13 @@ public void setApplicationContext(ApplicationContext applicationContext) throws
130131
}
131132

132133
@Override
133-
protected HandlerMapping createInstance() throws Exception {
134-
Collection<WebSocketConfigurer> webSocketConfigurers =
135-
((ListableBeanFactory) getBeanFactory()).getBeansOfType(WebSocketConfigurer.class).values();
136-
for (WebSocketConfigurer configurer : webSocketConfigurers) {
137-
configurer.registerWebSocketHandlers(this.registry);
134+
protected HandlerMapping createInstance() {
135+
BeanFactory beanFactory = getBeanFactory();
136+
if (beanFactory != null) {
137+
((ListableBeanFactory) beanFactory)
138+
.getBeansOfType(WebSocketConfigurer.class)
139+
.values()
140+
.forEach(configurer -> configurer.registerWebSocketHandlers(this.registry));
138141
}
139142
if (this.registry.requiresTaskScheduler()) {
140143
this.registry.setTaskScheduler(this.sockJsTaskScheduler);

0 commit comments

Comments
 (0)