Fix code smell in websocket and webflux modules (#2669)

* * Fix code smell in websocket and webflux modules

* * More fixes

* * Fix NPE in the `IntegrationHandlerResultHandler`
This commit is contained in:
Artem Bilan
2018-12-19 12:33:47 -05:00
committed by Gary Russell
parent e8bd31cc37
commit 8ce8bf9d73
9 changed files with 138 additions and 91 deletions

View File

@@ -53,7 +53,7 @@ public class PayloadTypeConvertingTransformer<T, U> extends AbstractPayloadTrans
}
@Override
protected void onInit() throws Exception {
protected void onInit() throws Exception { // NOSONAR thrown by super class
super.onInit();
Assert.notNull(this.converter, () -> getClass().getName() + " requires a Converter<Object, Object>");
}

View File

@@ -105,7 +105,7 @@ class DslScriptExecutingMessageProcessor
String filename = this.script.getFilename();
int index =
filename != null
? filename.lastIndexOf(".") + 1
? filename.lastIndexOf('.') + 1
: -1;
if (index < 1) {
throw new BeanCreationException(

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2017 the original author or authors.
* Copyright 2017-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -47,7 +47,8 @@ public class IntegrationHandlerResultHandler implements HandlerResultHandler, Or
@Override
@SuppressWarnings("unchecked")
public Mono<Void> handleResult(ServerWebExchange exchange, HandlerResult result) {
return (Mono<Void>) result.getReturnValue();
Object returnValue = result.getReturnValue();
return returnValue == null ? Mono.empty() : (Mono<Void>) returnValue;
}
@Override

View File

@@ -80,6 +80,8 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
private static final MediaType MEDIA_TYPE_APPLICATION_ALL = new MediaType("application");
private static final String UNCHECKED = "unchecked";
private static final List<HttpMethod> SAFE_METHODS = Arrays.asList(HttpMethod.GET, HttpMethod.HEAD);
private ServerCodecConfigurer codecConfigurer = ServerCodecConfigurer.create();
@@ -130,11 +132,6 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
return super.getComponentType().replaceFirst("http", "webflux");
}
@Override
protected void onInit() throws Exception {
super.onInit();
}
@Override
public Mono<Void> handle(ServerWebExchange exchange) {
return Mono.defer(() -> {
@@ -147,7 +144,6 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
});
}
@SuppressWarnings("unchecked")
private Mono<Void> doHandle(ServerWebExchange exchange) {
return extractRequestBody(exchange)
.doOnSubscribe(s -> this.activeCount.incrementAndGet())
@@ -170,7 +166,7 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
}
@SuppressWarnings("unchecked")
@SuppressWarnings(UNCHECKED)
private <T> Mono<T> extractRequestBody(ServerWebExchange exchange) {
ServerHttpRequest request = exchange.getRequest();
ServerHttpResponse response = exchange.getResponse();
@@ -201,7 +197,8 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
Class<?> resolvedType = bodyType.resolve();
ReactiveAdapter adapter = (resolvedType != null ? this.adapterRegistry.getAdapter(resolvedType) : null);
ReactiveAdapter adapter = (resolvedType != null ? this.adapterRegistry.getAdapter(resolvedType) :
null);
ResolvableType elementType = (adapter != null ? bodyType.getGeneric() : bodyType);
HttpMessageReader<?> httpMessageReader = this.codecConfigurer
@@ -237,40 +234,14 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
}
}
@SuppressWarnings("unchecked")
@SuppressWarnings(UNCHECKED)
private Mono<Tuple2<Message<Object>, RequestEntity<?>>> buildMessage(RequestEntity<?> httpEntity,
ServerWebExchange exchange) {
ServerHttpRequest request = exchange.getRequest();
HttpHeaders requestHeaders = request.getHeaders();
Map<String, Object> exchangeAttributes = exchange.getAttributes();
StandardEvaluationContext evaluationContext = createEvaluationContext();
evaluationContext.setVariable("requestAttributes", exchangeAttributes);
MultiValueMap<String, String> requestParams = request.getQueryParams();
evaluationContext.setVariable("requestParams", requestParams);
evaluationContext.setVariable("requestHeaders", requestHeaders);
if (!CollectionUtils.isEmpty(request.getCookies())) {
evaluationContext.setVariable("cookies", request.getCookies());
}
Map<String, String> pathVariables =
(Map<String, String>) exchangeAttributes.get(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
if (!CollectionUtils.isEmpty(pathVariables)) {
evaluationContext.setVariable("pathVariables", pathVariables);
}
Map<String, MultiValueMap<String, String>> matrixVariables =
(Map<String, MultiValueMap<String, String>>) exchangeAttributes
.get(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE);
if (!CollectionUtils.isEmpty(matrixVariables)) {
evaluationContext.setVariable("matrixVariables", matrixVariables);
}
evaluationContext.setRootObject(httpEntity);
StandardEvaluationContext evaluationContext = buildEvaluationContext(httpEntity, exchange);
Object payload;
if (getPayloadExpression() != null) {
payload = getPayloadExpression().getValue(evaluationContext);
@@ -310,10 +281,13 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
.copyHeaders(headers);
}
messageBuilder
.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_URL, request.getURI().toString())
.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_METHOD,
request.getMethod().toString());
messageBuilder.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_URL,
request.getURI().toString());
HttpMethod httpMethod = request.getMethod();
if (httpMethod != null) {
messageBuilder.setHeader(org.springframework.integration.http.HttpHeaders.REQUEST_METHOD,
httpMethod.toString());
}
return exchange.getPrincipal()
.map(principal ->
@@ -324,6 +298,41 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
.zipWith(Mono.just(httpEntity));
}
@SuppressWarnings(UNCHECKED)
private StandardEvaluationContext buildEvaluationContext(RequestEntity<?> httpEntity, ServerWebExchange exchange) {
ServerHttpRequest request = exchange.getRequest();
HttpHeaders requestHeaders = request.getHeaders();
MultiValueMap<String, String> requestParams = request.getQueryParams();
Map<String, Object> exchangeAttributes = exchange.getAttributes();
StandardEvaluationContext evaluationContext = createEvaluationContext();
evaluationContext.setVariable("requestAttributes", exchangeAttributes);
evaluationContext.setVariable("requestParams", requestParams);
evaluationContext.setVariable("requestHeaders", requestHeaders);
if (!CollectionUtils.isEmpty(request.getCookies())) {
evaluationContext.setVariable("cookies", request.getCookies());
}
Map<String, String> pathVariables =
(Map<String, String>) exchangeAttributes.get(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
if (!CollectionUtils.isEmpty(pathVariables)) {
evaluationContext.setVariable("pathVariables", pathVariables);
}
Map<String, MultiValueMap<String, String>> matrixVariables =
(Map<String, MultiValueMap<String, String>>) exchangeAttributes
.get(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE);
if (!CollectionUtils.isEmpty(matrixVariables)) {
evaluationContext.setVariable("matrixVariables", matrixVariables);
}
evaluationContext.setRootObject(httpEntity);
return evaluationContext;
}
private Mono<Void> populateResponse(ServerWebExchange exchange, Message<?> replyMessage) {
ServerHttpResponse response = exchange.getResponse();
getHeaderMapper().fromHeaders(replyMessage.getHeaders(), response.getHeaders());
@@ -379,7 +388,7 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
}
}
@SuppressWarnings("unchecked")
@SuppressWarnings(UNCHECKED)
private Mono<Void> writeResponseBody(ServerWebExchange exchange, Object body) {
ResolvableType bodyType = ResolvableType.forInstance(body);
ReactiveAdapter adapter = this.adapterRegistry.getAdapter(bodyType.resolve(), body);
@@ -473,7 +482,7 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
return (mediaTypes.isEmpty() ? Collections.singletonList(MediaType.ALL) : mediaTypes);
}
@SuppressWarnings("unchecked")
@SuppressWarnings(UNCHECKED)
private List<MediaType> getProducibleTypes(ServerWebExchange exchange,
Supplier<List<MediaType>> producibleTypesSupplier) {
@@ -482,9 +491,9 @@ public class WebFluxInboundEndpoint extends BaseHttpInboundEndpoint implements W
}
private MediaType selectMoreSpecificMediaType(MediaType acceptable, MediaType producible) {
producible = producible.copyQualityValue(acceptable);
MediaType producibleToUse = producible.copyQualityValue(acceptable);
Comparator<MediaType> comparator = MediaType.SPECIFICITY_COMPARATOR;
return (comparator.compare(acceptable, producible) <= 0 ? acceptable : producible);
return (comparator.compare(acceptable, producibleToUse) <= 0 ? acceptable : producibleToUse);
}

View File

@@ -119,11 +119,11 @@ public class WebFluxIntegrationRequestMappingHandlerMapping extends RequestMappi
@Override
protected void detectHandlerMethods(Object handler) {
if (handler instanceof String) {
handler = getApplicationContext().getBean((String) handler);
handler = getApplicationContext().getBean((String) handler); // NOSONAR never null
}
RequestMappingInfo mapping = getMappingForEndpoint((WebFluxInboundEndpoint) handler);
if (mapping != null) {
registerMapping(mapping, handler, HANDLER_METHOD);
registerMapping(mapping, handler, HANDLER_METHOD); // NOSONAR never null
}
}

View File

@@ -159,7 +159,7 @@ public class WebFluxRequestExecutingMessageHandler extends AbstractHttpRequestEx
.headers(headers -> headers.putAll(httpRequest.getHeaders()));
if (httpRequest.hasBody()) {
requestSpec.body(BodyInserters.fromObject(httpRequest.getBody()));
requestSpec.body(BodyInserters.fromObject(httpRequest.getBody())); // NOSONAR protected with hasBody()
}
Mono<ClientResponse> responseMono =

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2017 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,12 +16,11 @@
package org.springframework.integration.websocket.config;
import java.util.Collection;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.ListableBeanFactory;
import org.springframework.beans.factory.config.AbstractFactoryBean;
import org.springframework.beans.factory.config.BeanDefinition;
@@ -50,10 +49,12 @@ import org.springframework.web.socket.config.annotation.WebSocketConfigurer;
*/
public class WebSocketIntegrationConfigurationInitializer implements IntegrationConfigurationInitializer {
private static final Log logger = LogFactory.getLog(WebSocketIntegrationConfigurationInitializer.class);
private static final Log logger = // NOSONAR lower case
LogFactory.getLog(WebSocketIntegrationConfigurationInitializer.class);
private static final boolean servletPresent = ClassUtils.isPresent("javax.servlet.Servlet",
WebSocketIntegrationConfigurationInitializer.class.getClassLoader());
private static final boolean servletPresent = // NOSONAR lower case
ClassUtils.isPresent("javax.servlet.Servlet",
WebSocketIntegrationConfigurationInitializer.class.getClassLoader());
private static final String WEB_SOCKET_HANDLER_MAPPING_BEAN_NAME = "integrationWebSocketHandlerMapping";
@@ -130,11 +131,13 @@ public class WebSocketIntegrationConfigurationInitializer implements Integration
}
@Override
protected HandlerMapping createInstance() throws Exception {
Collection<WebSocketConfigurer> webSocketConfigurers =
((ListableBeanFactory) getBeanFactory()).getBeansOfType(WebSocketConfigurer.class).values();
for (WebSocketConfigurer configurer : webSocketConfigurers) {
configurer.registerWebSocketHandlers(this.registry);
protected HandlerMapping createInstance() {
BeanFactory beanFactory = getBeanFactory();
if (beanFactory != null) {
((ListableBeanFactory) beanFactory)
.getBeansOfType(WebSocketConfigurer.class)
.values()
.forEach(configurer -> configurer.registerWebSocketHandlers(this.registry));
}
if (this.registry.requiresTaskScheduler()) {
this.registry.setTaskScheduler(this.sockJsTaskScheduler);

View File

@@ -220,7 +220,7 @@ public class WebSocketInboundChannelAdapter extends MessageProducerSupport
}
@Override
public void afterSessionStarted(WebSocketSession session) throws Exception {
public void afterSessionStarted(WebSocketSession session) throws Exception { // NOSONAR Thrown from the delegate
if (isActive()) {
SubProtocolHandler protocolHandler = this.subProtocolHandlerRegistry.findProtocolHandler(session);
protocolHandler.afterSessionStarted(session, this.subProtocolHandlerChannel);
@@ -237,7 +237,8 @@ public class WebSocketInboundChannelAdapter extends MessageProducerSupport
}
@Override
public void afterSessionEnded(WebSocketSession session, CloseStatus closeStatus) throws Exception {
public void afterSessionEnded(WebSocketSession session, CloseStatus closeStatus)
throws Exception { // NOSONAR Thrown from the delegate
if (isActive()) {
this.subProtocolHandlerRegistry.findProtocolHandler(session)
.afterSessionEnded(session, closeStatus, this.subProtocolHandlerChannel);
@@ -245,7 +246,8 @@ public class WebSocketInboundChannelAdapter extends MessageProducerSupport
}
@Override
public void onMessage(WebSocketSession session, WebSocketMessage<?> webSocketMessage) throws Exception {
public void onMessage(WebSocketSession session, WebSocketMessage<?> webSocketMessage)
throws Exception { // NOSONAR Thrown from the delegate
if (isActive()) {
this.subProtocolHandlerRegistry.findProtocolHandler(session)
.handleMessageFromClient(session, webSocketMessage, this.subProtocolHandlerChannel);
@@ -281,23 +283,13 @@ public class WebSocketInboundChannelAdapter extends MessageProducerSupport
}
@SuppressWarnings("unchecked")
private void handleMessageAndSend(Message<?> message) throws Exception {
private void handleMessageAndSend(final Message<?> message) {
SimpMessageHeaderAccessor headerAccessor = SimpMessageHeaderAccessor.wrap(message);
StompCommand stompCommand = (StompCommand) headerAccessor.getHeader("stompCommand");
SimpMessageType messageType = headerAccessor.getMessageType();
if ((messageType == null || SimpMessageType.MESSAGE.equals(messageType)
|| (SimpMessageType.CONNECT.equals(messageType) && !this.useBroker)
|| StompCommand.CONNECTED.equals(stompCommand)
|| StompCommand.RECEIPT.equals(stompCommand))
&& !checkDestinationPrefix(headerAccessor.getDestination())) {
if (isProcessingTypeOrCommand(headerAccessor, stompCommand, messageType)) {
if (SimpMessageType.CONNECT.equals(messageType)) {
String sessionId = headerAccessor.getSessionId();
SimpMessageHeaderAccessor connectAck = SimpMessageHeaderAccessor.create(SimpMessageType.CONNECT_ACK);
connectAck.setSessionId(sessionId);
connectAck.setHeader(SimpMessageHeaderAccessor.CONNECT_MESSAGE_HEADER, message);
Message<byte[]> ackMessage = MessageBuilder.createMessage(EMPTY_PAYLOAD, connectAck.getMessageHeaders());
WebSocketSession session = this.webSocketContainer.getSession(sessionId);
this.subProtocolHandlerRegistry.findProtocolHandler(session).handleMessageToClient(session, ackMessage);
produceConnectAckMessage(message, headerAccessor);
}
else if (StompCommand.CONNECTED.equals(stompCommand)) {
this.eventPublisher.publishEvent(new SessionConnectedEvent(this, (Message<byte[]>) message));
@@ -306,9 +298,7 @@ public class WebSocketInboundChannelAdapter extends MessageProducerSupport
this.eventPublisher.publishEvent(new ReceiptEvent(this, (Message<byte[]>) message));
}
else {
headerAccessor.removeHeader(SimpMessageHeaderAccessor.NATIVE_HEADERS);
Object payload = this.messageConverter.fromMessage(message, this.payloadType.get());
sendMessage(getMessageBuilderFactory().withPayload(payload).copyHeaders(headerAccessor.toMap()).build());
produceMessage(message, headerAccessor);
}
}
else {
@@ -324,19 +314,56 @@ public class WebSocketInboundChannelAdapter extends MessageProducerSupport
}
}
private boolean isProcessingTypeOrCommand(SimpMessageHeaderAccessor headerAccessor, StompCommand stompCommand,
SimpMessageType messageType) {
return (messageType == null // NOSONAR pretty simple logic
|| SimpMessageType.MESSAGE.equals(messageType)
|| (SimpMessageType.CONNECT.equals(messageType) && !this.useBroker)
|| StompCommand.CONNECTED.equals(stompCommand)
|| StompCommand.RECEIPT.equals(stompCommand))
&& !checkDestinationPrefix(headerAccessor.getDestination());
}
private boolean checkDestinationPrefix(String destination) {
if (this.useBroker) {
Collection<String> destinationPrefixes = this.brokerHandler.getDestinationPrefixes();
if ((destination == null) || CollectionUtils.isEmpty(destinationPrefixes)) {
return false;
}
for (String prefix : destinationPrefixes) {
if (destination.startsWith(prefix)) {
return true;
}
}
return destinationPrefixes.stream().anyMatch(destination::startsWith);
}
return false;
}
private void produceConnectAckMessage(Message<?> message, SimpMessageHeaderAccessor headerAccessor) {
String sessionId = headerAccessor.getSessionId();
SimpMessageHeaderAccessor connectAck = SimpMessageHeaderAccessor.create(SimpMessageType.CONNECT_ACK);
connectAck.setSessionId(sessionId);
connectAck.setHeader(SimpMessageHeaderAccessor.CONNECT_MESSAGE_HEADER, message);
Message<byte[]> ackMessage = MessageBuilder.createMessage(EMPTY_PAYLOAD, connectAck.getMessageHeaders());
WebSocketSession session = this.webSocketContainer.getSession(sessionId);
try {
this.subProtocolHandlerRegistry.findProtocolHandler(session).handleMessageToClient(session, ackMessage);
}
catch (Exception e) {
throw new MessageHandlingException(message, "Error sending connect ack message", e);
}
}
private void produceMessage(Message<?> message, SimpMessageHeaderAccessor headerAccessor) {
headerAccessor.removeHeader(SimpMessageHeaderAccessor.NATIVE_HEADERS);
Object payload = this.messageConverter.fromMessage(message, this.payloadType.get());
Assert.state(payload != null,
() -> "The message converter '" + this.messageConverter +
"' produced no payload for message '" + message +
"' and expected payload type: " + this.payloadType.get());
Message<Object> messageToSend =
getMessageBuilderFactory()
.withPayload(payload)
.copyHeaders(headerAccessor.toMap())
.build();
sendMessage(messageToSend);
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2017 the original author or authors.
* Copyright 2014-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -43,11 +43,12 @@ import org.springframework.web.socket.handler.SessionLimitExceededException;
/**
* @author Artem Bilan
*
* @since 4.1
*/
public class WebSocketOutboundMessageHandler extends AbstractMessageHandler {
private final List<MessageConverter> defaultConverters = new ArrayList<MessageConverter>(3);
private final List<MessageConverter> defaultConverters = new ArrayList<>(3);
{
this.defaultConverters.add(new StringMessageConverter());
@@ -79,13 +80,14 @@ public class WebSocketOutboundMessageHandler extends AbstractMessageHandler {
public WebSocketOutboundMessageHandler(IntegrationWebSocketContainer webSocketContainer,
SubProtocolHandlerRegistry protocolHandlerRegistry) {
Assert.notNull(webSocketContainer, "'webSocketContainer' must not be null");
Assert.notNull(protocolHandlerRegistry, "'protocolHandlerRegistry' must not be null");
this.webSocketContainer = webSocketContainer;
this.client = webSocketContainer instanceof ClientWebSocketContainer;
this.subProtocolHandlerRegistry = protocolHandlerRegistry;
List<String> subProtocols = protocolHandlerRegistry.getSubProtocols();
this.webSocketContainer.addSupportedProtocols(subProtocols.toArray(new String[subProtocols.size()]));
this.webSocketContainer.addSupportedProtocols(subProtocols.toArray(new String[0]));
}
/**
@@ -95,7 +97,7 @@ public class WebSocketOutboundMessageHandler extends AbstractMessageHandler {
*/
public void setMessageConverters(List<MessageConverter> messageConverters) {
Assert.noNullElements(messageConverters.toArray(), "'messageConverters' must not contain null entries");
this.messageConverters = new ArrayList<MessageConverter>(messageConverters);
this.messageConverters = new ArrayList<>(messageConverters);
}
@@ -147,7 +149,12 @@ public class WebSocketOutboundMessageHandler extends AbstractMessageHandler {
SimpMessageHeaderAccessor headers = SimpMessageHeaderAccessor.wrap(message);
headers.setLeaveMutable(true);
headers.setMessageTypeIfNotSet(SimpMessageType.MESSAGE);
Message<?> messageToSend = this.messageConverter.toMessage(message.getPayload(), headers.getMessageHeaders());
Object payload = message.getPayload();
Message<?> messageToSend =
this.messageConverter.toMessage(payload, headers.getMessageHeaders());
Assert.state(messageToSend != null,
() -> "The message converter '" + this.messageConverter +
"' produced no message to send based on the request message: '" + message + "'");
this.subProtocolHandlerRegistry.findProtocolHandler(session).handleMessageToClient(session, messageToSend);
}
catch (SessionLimitExceededException ex) {