From cfeaecaae85566aeb3ef81b4046b327a15a29ed1 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Fri, 14 Oct 2022 16:47:00 -0400 Subject: [PATCH] GH-3421: Resolve `throws Exception;` in the API Fixes https://github.com/spring-projects/spring-integration/issues/3421 Remove `throws Exception;` from production code to honor the rule `Generic exceptions should never be thrown` which is enabled on SonarQube * Rework affected usages to `try..catch` with throwing respective runtime exception or just logging * Some other refactoring for the affected classes --- .../AbstractMailMessageTransformer.java | 23 ++------ .../transformer/MailToStringTransformer.java | 46 +++++++++------- .../websocket/WebSocketListener.java | 13 ++--- .../WebSocketInboundChannelAdapter.java | 55 ++++++++++++------- 4 files changed, 72 insertions(+), 65 deletions(-) diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/AbstractMailMessageTransformer.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/AbstractMailMessageTransformer.java index 1ee8d4dd7a..b2ecf565da 100644 --- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/AbstractMailMessageTransformer.java +++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/AbstractMailMessageTransformer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -39,8 +39,7 @@ import org.springframework.messaging.Message; * @author Gary Russell * @author Artem Bilan */ -public abstract class AbstractMailMessageTransformer implements Transformer, - BeanFactoryAware { +public abstract class AbstractMailMessageTransformer implements Transformer, BeanFactoryAware { private BeanFactory beanFactory; @@ -48,7 +47,6 @@ public abstract class AbstractMailMessageTransformer implements Transformer, private boolean messageBuilderFactorySet; - @Override public final void setBeanFactory(BeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -67,28 +65,19 @@ public abstract class AbstractMailMessageTransformer implements Transformer, @Override public Message transform(Message message) { Object payload = message.getPayload(); - if (!(payload instanceof jakarta.mail.Message)) { + if (!(payload instanceof jakarta.mail.Message mailMessage)) { throw new MessageTransformationException(message, getClass().getSimpleName() + " requires a jakarta.mail.Message payload"); } - jakarta.mail.Message mailMessage = (jakarta.mail.Message) payload; AbstractIntegrationMessageBuilder builder; - try { - builder = this.doTransform(mailMessage); - } - catch (Exception e) { - throw new MessageTransformationException(message, "failed to transform mail message", e); - } + builder = doTransform(mailMessage); if (builder == null) { throw new MessageTransformationException(message, "failed to transform mail message"); } - builder.copyHeaders(extractHeaderMapFromMailMessage(mailMessage)); - return builder.build(); + return builder.copyHeaders(extractHeaderMapFromMailMessage(mailMessage)).build(); } - protected abstract AbstractIntegrationMessageBuilder doTransform(jakarta.mail.Message mailMessage) - throws Exception; // NOSONAR - + protected abstract AbstractIntegrationMessageBuilder doTransform(jakarta.mail.Message mailMessage); private static Map extractHeaderMapFromMailMessage(jakarta.mail.Message mailMessage) { return MailUtils.extractStandardHeaders(mailMessage); diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/MailToStringTransformer.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/MailToStringTransformer.java index c2c2b042d7..1fd729e49f 100644 --- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/MailToStringTransformer.java +++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/MailToStringTransformer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -23,6 +23,7 @@ import jakarta.mail.Multipart; import jakarta.mail.Part; import org.springframework.integration.support.AbstractIntegrationMessageBuilder; +import org.springframework.integration.transformer.MessageTransformationException; import org.springframework.util.Assert; /** @@ -51,27 +52,34 @@ public class MailToStringTransformer extends AbstractMailMessageTransformer doTransform(jakarta.mail.Message mailMessage) - throws Exception { // NOSONAR + protected AbstractIntegrationMessageBuilder doTransform(jakarta.mail.Message mailMessage) { + try { + String payload; + Object content = mailMessage.getContent(); + if (content instanceof String value) { + payload = value; + } + else if (content instanceof Multipart multipart) { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + multipart.writeTo(outputStream); + payload = outputStream.toString(this.charset); + } + else if (content instanceof Part part) { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + part.writeTo(outputStream); + payload = outputStream.toString(this.charset); + } + else { + throw new IllegalArgumentException("failed to transform contentType [" + + mailMessage.getContentType() + "] to String."); + } - Object content = mailMessage.getContent(); - if (content instanceof String) { - return this.getMessageBuilderFactory().withPayload((String) content); + return getMessageBuilderFactory().withPayload(payload); } - if (content instanceof Multipart) { - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - ((Multipart) content).writeTo(outputStream); - return this.getMessageBuilderFactory().withPayload( - new String(outputStream.toByteArray(), this.charset)); + catch (Exception ex) { + throw new MessageTransformationException("Cannot transform mail message", ex); } - else if (content instanceof Part) { - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - ((Part) content).writeTo(outputStream); - return this.getMessageBuilderFactory().withPayload( - new String(outputStream.toByteArray(), this.charset)); - } - throw new IllegalArgumentException("failed to transform contentType [" - + mailMessage.getContentType() + "] to String."); + } } diff --git a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/WebSocketListener.java b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/WebSocketListener.java index fd55ce5e05..642900ac66 100644 --- a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/WebSocketListener.java +++ b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/WebSocketListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2022 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. @@ -42,25 +42,20 @@ public interface WebSocketListener extends SubProtocolCapable { * Handle the received {@link WebSocketMessage}. * @param session the WebSocket session * @param message the WebSocket message - * @throws Exception the 'onMessage' Exception */ - void onMessage(WebSocketSession session, WebSocketMessage message) - throws Exception; // NOSONAR Remove in 5.2 + void onMessage(WebSocketSession session, WebSocketMessage message); /** * Invoked after a {@link WebSocketSession} has started. * @param session the WebSocket session - * @throws Exception the 'afterSessionStarted' Exception */ - void afterSessionStarted(WebSocketSession session) throws Exception; // NOSONAR Remove in 5.2 + void afterSessionStarted(WebSocketSession session); /** * Invoked after a {@link WebSocketSession} has ended. * @param session the WebSocket session * @param closeStatus the reason why the session was closed - * @throws Exception the 'afterSessionEnded' Exception */ - void afterSessionEnded(WebSocketSession session, CloseStatus closeStatus) - throws Exception; // NOSONAR Remove in 5.2 + void afterSessionEnded(WebSocketSession session, CloseStatus closeStatus); } diff --git a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/inbound/WebSocketInboundChannelAdapter.java b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/inbound/WebSocketInboundChannelAdapter.java index b00bf01dfa..5c05d23faf 100644 --- a/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/inbound/WebSocketInboundChannelAdapter.java +++ b/spring-integration-websocket/src/main/java/org/springframework/integration/websocket/inbound/WebSocketInboundChannelAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2021 the original author or authors. + * Copyright 2014-2022 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. @@ -224,39 +224,54 @@ public class WebSocketInboundChannelAdapter extends MessageProducerSupport } @Override - public void afterSessionStarted(WebSocketSession session) throws Exception { // NOSONAR Thrown from the delegate + public void afterSessionStarted(WebSocketSession session) { if (isActive()) { - SubProtocolHandler protocolHandler = this.subProtocolHandlerRegistry.findProtocolHandler(session); - protocolHandler.afterSessionStarted(session, this.subProtocolHandlerChannel); - if (!this.server && protocolHandler instanceof StompSubProtocolHandler) { - StompHeaderAccessor accessor = StompHeaderAccessor.create(StompCommand.CONNECT); - accessor.setSessionId(session.getId()); - accessor.setLeaveMutable(true); - accessor.setAcceptVersion("1.1,1.2"); + try { + SubProtocolHandler protocolHandler = this.subProtocolHandlerRegistry.findProtocolHandler(session); + protocolHandler.afterSessionStarted(session, this.subProtocolHandlerChannel); + if (!this.server && protocolHandler instanceof StompSubProtocolHandler) { + StompHeaderAccessor accessor = StompHeaderAccessor.create(StompCommand.CONNECT); + accessor.setSessionId(session.getId()); + accessor.setLeaveMutable(true); + accessor.setAcceptVersion("1.1,1.2"); - Message connectMessage = MessageBuilder.createMessage(EMPTY_PAYLOAD, accessor.getMessageHeaders()); - protocolHandler.handleMessageToClient(session, connectMessage); + Message connectMessage = + MessageBuilder.createMessage(EMPTY_PAYLOAD, accessor.getMessageHeaders()); + protocolHandler.handleMessageToClient(session, connectMessage); + } + } + catch (Exception ex) { + logger.error(ex, () -> "WebSocketHandler.afterConnectionEstablished threw exception in " + this); } } } @Override - public void afterSessionEnded(WebSocketSession session, CloseStatus closeStatus) - throws Exception { // NOSONAR Thrown from the delegate + public void afterSessionEnded(WebSocketSession session, CloseStatus closeStatus) { if (isActive()) { - this.subProtocolHandlerRegistry.findProtocolHandler(session) - .afterSessionEnded(session, closeStatus, this.subProtocolHandlerChannel); + try { + this.subProtocolHandlerRegistry.findProtocolHandler(session) + .afterSessionEnded(session, closeStatus, this.subProtocolHandlerChannel); + } + catch (Exception ex) { + logger.warn(ex, () -> "Unhandled exception after connection closed for " + this); + } } } @Override - public void onMessage(WebSocketSession session, WebSocketMessage webSocketMessage) - throws Exception { // NOSONAR Thrown from the delegate - + public void onMessage(WebSocketSession session, WebSocketMessage webSocketMessage) { if (isActive()) { - this.subProtocolHandlerRegistry.findProtocolHandler(session) - .handleMessageFromClient(session, webSocketMessage, this.subProtocolHandlerChannel); + try { + this.subProtocolHandlerRegistry.findProtocolHandler(session) + .handleMessageFromClient(session, webSocketMessage, this.subProtocolHandlerChannel); + } + catch (Exception ex) { + logger.error(ex, + () -> "SubProtocolHandler.handleMessageFromClient threw an exception on message: " + + webSocketMessage + " in " + this); + } } }