Fix new Sonar smell and some other in JMX module (#2716)

* Fix new Sonar smell and some other in JMX module

* * Fix issues after testing

Author: artembilan

spring-integration-core/src/main/java/org/springframework/integration/dsl/IntegrationComponentSpec.java

@@ -79,18 +79,28 @@ public Class<?> getObjectType() {
 	}
 
 	@Override
-	protected T createInstance() throws Exception {
+	protected T createInstance() {
 		T instance = get();
 		if (instance instanceof InitializingBean) {
-			((InitializingBean) instance).afterPropertiesSet();
+			try {
+				((InitializingBean) instance).afterPropertiesSet();
+			}
+			catch (Exception e) {
+				throw new IllegalStateException("Cannot initialize bean: " + instance, e);
+			}
 		}
 		return instance;
 	}
 
 	@Override
-	protected void destroyInstance(T instance) throws Exception {
+	protected void destroyInstance(T instance)  {
 		if (instance instanceof DisposableBean) {
-			((DisposableBean) instance).destroy();
+			try {
+				((DisposableBean) instance).destroy();
+			}
+			catch (Exception e) {
+				throw new IllegalStateException("Cannot destroy bean: " + instance, e);
+			}
 		}
 	}
 
@@ -145,7 +155,7 @@ public int getPhase() {
 	}
 
 	@SuppressWarnings("unchecked")
-	protected final S _this() {
+	protected final S _this() { // NOSONAR
 		return (S) this;
 	}
 

spring-integration-jmx/src/main/java/org/springframework/integration/jmx/DefaultMBeanObjectConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2016 the original author or authors.
+ * Copyright 2013-2019 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.
@@ -35,14 +35,18 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.util.Assert;
+
 /**
  * @author Stuart Williams
+ * @author Artem Bilan
+ *
  * @since 3.0
  *
  */
 public class DefaultMBeanObjectConverter implements MBeanObjectConverter {
 
-	private static final Log log = LogFactory.getLog(DefaultMBeanObjectConverter.class);
+	private static final Log LOGGER = LogFactory.getLog(DefaultMBeanObjectConverter.class);
 
 	private final MBeanAttributeFilter filter;
 
@@ -51,12 +55,13 @@ public DefaultMBeanObjectConverter() {
 	}
 
 	public DefaultMBeanObjectConverter(MBeanAttributeFilter filter) {
+		Assert.notNull(filter, "'filter' must not be null.");
 		this.filter = filter;
 	}
 
 	@Override
 	public Object convert(MBeanServerConnection connection, ObjectInstance instance) {
-		Map<String, Object> attributeMap = new HashMap<String, Object>();
+		Map<String, Object> attributeMap = new HashMap<>();
 
 		try {
 			ObjectName objName = instance.getObjectName();
@@ -81,8 +86,8 @@ public Object convert(MBeanServerConnection connection, ObjectInstance instance)
 					// N.B. standard MemoryUsage MBeans will throw an exception when some
 					// measurement is unsupported. Logging at trace rather than debug to
 					// avoid confusion.
-					if (log.isTraceEnabled()) {
-						log.trace("Error getting attribute '" + attrInfo.getName() + "' on '" + objName + "'", e);
+					if (LOGGER.isTraceEnabled()) {
+						LOGGER.trace("Error getting attribute '" + attrInfo.getName() + "' on '" + objName + "'", e);
 					}
 
 					// try to unwrap the exception somewhat; not sure this is ideal
@@ -104,90 +109,95 @@ public Object convert(MBeanServerConnection connection, ObjectInstance instance)
 		return attributeMap;
 	}
 
-	/**
-	 * @param input
-	 * @return recursively mapped object
-	 */
 	private Object checkAndConvert(Object input) {
-		if (input == null) {
+		Object converted = null;
+		if (input instanceof CompositeData) {
+			converted = convertFromCompositeData((CompositeData) input);
+		}
+		else if (input instanceof TabularData) {
+			converted = convertFromTabularData((TabularData) input);
+		}
+		else if (input != null && input.getClass().isArray()) {
+			converted = convertFromArray(input);
+		}
+
+		if (converted != null) {
+			return converted;
+		}
+		else {
 			return input;
 		}
-		else if (input.getClass().isArray()) {
-
-			if (CompositeData.class.isAssignableFrom(input.getClass().getComponentType())) {
-				List<Object> converted = new ArrayList<Object>();
-				int length = Array.getLength(input);
-				for (int i = 0; i < length; i++) {
-					Object value = checkAndConvert(Array.get(input, i));
-					converted.add(value);
-				}
-				return converted;
-			}
-			if (TabularData.class.isAssignableFrom(input.getClass().getComponentType())) {
-				// TODO haven't hit this yet, but expect to
-				log.warn("TabularData.isAssignableFrom(getComponentType) for " + input.toString());
+	}
+
+	private Object convertFromArray(Object input) {
+		if (CompositeData.class.isAssignableFrom(input.getClass().getComponentType())) {
+			List<Object> converted = new ArrayList<>();
+			int length = Array.getLength(input);
+			for (int i = 0; i < length; i++) {
+				Object value = checkAndConvert(Array.get(input, i));
+				converted.add(value);
 			}
+			return converted;
+		}
+		if (TabularData.class.isAssignableFrom(input.getClass().getComponentType())) {
+			// TODO haven't hit this yet, but expect to
+			LOGGER.warn("TabularData.isAssignableFrom(getComponentType) for " + input.toString());
 		}
-		else if (input instanceof CompositeData) {
-			CompositeData data = (CompositeData) input;
+		return null;
+	}
 
-			if (data.getCompositeType().isArray()) {
-				// TODO? I haven't found an example where this gets thrown - but need to test it on Tomcat/Jetty or
-				// something
-				log.warn("(data.getCompositeType().isArray for " + input.toString());
-			}
-			else {
-				Map<String, Object> returnable = new HashMap<String, Object>();
-				Set<String> keys = data.getCompositeType().keySet();
-				for (String key : keys) {
-					// we don't need to repeat name of this as an attribute
-					if ("ObjectName".equals(key)) {
-						continue;
-					}
-					Object value = checkAndConvert(data.get(key));
-					returnable.put(key, value);
+	private Object convertFromCompositeData(CompositeData data) {
+		if (data.getCompositeType().isArray()) {
+			// TODO? I haven't found an example where this gets thrown - but need to test it on Tomcat/Jetty or
+			// something
+			LOGGER.warn("(data.getCompositeType().isArray for " + data.toString());
+			return null;
+		}
+		else {
+			Map<String, Object> returnable = new HashMap<>();
+			Set<String> keys = data.getCompositeType().keySet();
+			for (String key : keys) {
+				// we don't need to repeat name of this as an attribute
+				if ("ObjectName".equals(key)) {
+					continue;
 				}
-				return returnable;
+				Object value = checkAndConvert(data.get(key));
+				returnable.put(key, value);
 			}
+			return returnable;
 		}
-		else if (input instanceof TabularData) {
-			TabularData data = (TabularData) input;
+	}
 
-			if (data.getTabularType().isArray()) {
-				// TODO? I haven't found an example where this gets thrown, so might not be required
-				log.warn("TabularData.isArray for " + input.toString());
-			}
-			else {
-
-				Map<Object, Object> returnable = new HashMap<Object, Object>();
-				@SuppressWarnings("unchecked")
-				Set<List<?>> keySet = (Set<List<?>>) data.keySet();
-				for (List<?> keys : keySet) {
-					CompositeData cd = data.get(keys.toArray());
-					Object value = checkAndConvert(cd);
-
-					if (keys.size() == 1 && (value instanceof Map) && ((Map<?, ?>) value).size() == 2) {
-
-						Object actualKey = keys.get(0);
-						Map<?, ?> valueMap = (Map<?, ?>) value;
-
-						if (valueMap.containsKey("key") && valueMap.containsKey("value")
-								&& actualKey.equals(valueMap.get("key"))) {
-							returnable.put(valueMap.get("key"), valueMap.get("value"));
-						}
-						else {
-							returnable.put(actualKey, value);
-						}
+	private Object convertFromTabularData(TabularData data) {
+		if (data.getTabularType().isArray()) {
+			// TODO? I haven't found an example where this gets thrown, so might not be required
+			LOGGER.warn("TabularData.isArray for " + data.toString());
+			return null;
+		}
+		else {
+			Map<Object, Object> returnable = new HashMap<>();
+			@SuppressWarnings("unchecked")
+			Set<List<?>> keySet = (Set<List<?>>) data.keySet();
+			for (List<?> keys : keySet) {
+				CompositeData cd = data.get(keys.toArray());
+				Object value = checkAndConvert(cd);
+				if (keys.size() == 1 && (value instanceof Map) && ((Map<?, ?>) value).size() == 2) {
+					Object actualKey = keys.get(0);
+					Map<?, ?> valueMap = (Map<?, ?>) value;
+					if (valueMap.containsKey("key") && valueMap.containsKey("value")
+							&& actualKey.equals(valueMap.get("key"))) {
+						returnable.put(valueMap.get("key"), valueMap.get("value"));
 					}
 					else {
-						returnable.put(keys, value);
+						returnable.put(actualKey, value);
 					}
 				}
-
-				return returnable;
+				else {
+					returnable.put(keys, value);
+				}
 			}
+			return returnable;
 		}
-
-		return input;
 	}
+
 }

spring-integration-jmx/src/main/java/org/springframework/integration/jmx/DefaultNotificationMapper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -22,6 +22,7 @@
 import javax.management.ObjectName;
 
 import org.springframework.integration.mapping.OutboundMessageMapper;
+import org.springframework.lang.Nullable;
 import org.springframework.messaging.Message;
 import org.springframework.util.Assert;
 
@@ -32,32 +33,34 @@
  * 'userData' of the Notification instance.
  *
  * @author Mark Fisher
+ * @author Artem Bilan
+ *
  * @since 2.0
  */
 class DefaultNotificationMapper implements OutboundMessageMapper<Notification> {
 
 	private final ObjectName sourceObjectName;
 
+	@Nullable
 	private final String defaultNotificationType;
 
 	private final AtomicLong sequence = new AtomicLong();
 
 
-	DefaultNotificationMapper(ObjectName sourceObjectName, String defaultNotificationType) {
+	DefaultNotificationMapper(ObjectName sourceObjectName, @Nullable String defaultNotificationType) {
 		this.sourceObjectName = sourceObjectName;
 		this.defaultNotificationType = defaultNotificationType;
 	}
 
 
-	public Notification fromMessage(Message<?> message) throws Exception {
-		String type = this.resolveNotificationType(message);
-		Assert.hasText(type,
-				"No notification type header is available, and no default has been provided.");
-		Object payload = (message != null) ? message.getPayload() : null;
+	public Notification fromMessage(Message<?> message) {
+		String type = resolveNotificationType(message);
+		Assert.hasText(type, "No notification type header is available, and no default has been provided.");
+		Object payload = message.getPayload();
 		String notificationMessage = (payload instanceof String) ? (String) payload : null;
 		Notification notification = new Notification(type, this.sourceObjectName,
 				this.sequence.incrementAndGet(), System.currentTimeMillis(), notificationMessage);
-		if (payload != null && !(payload instanceof String)) {
+		if (!(payload instanceof String)) {
 			notification.setUserData(payload);
 		}
 		return notification;

spring-integration-jmx/src/main/java/org/springframework/integration/jmx/NotificationPublishingMessageHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -35,6 +35,7 @@
 import org.springframework.jmx.export.notification.NotificationPublisher;
 import org.springframework.jmx.export.notification.NotificationPublisherAware;
 import org.springframework.jmx.support.ObjectNameManager;
+import org.springframework.lang.Nullable;
 import org.springframework.messaging.Message;
 import org.springframework.util.Assert;
 
@@ -55,11 +56,12 @@ public class NotificationPublishingMessageHandler extends AbstractMessageHandler
 
 	private final PublisherDelegate delegate = new PublisherDelegate();
 
-	private volatile OutboundMessageMapper<Notification> notificationMapper;
-
 	private final ObjectName objectName;
 
-	private volatile String defaultNotificationType;
+	private String defaultNotificationType;
+
+	@Nullable
+	private OutboundMessageMapper<Notification> notificationMapper;
 
 
 	public NotificationPublishingMessageHandler(ObjectName objectName) {
@@ -85,7 +87,7 @@ public NotificationPublishingMessageHandler(String objectName) {
 	 * will be passed as the 'userData' of the Notification.
 	 * @param notificationMapper The notification mapper.
 	 */
-	public void setNotificationMapper(OutboundMessageMapper<Notification> notificationMapper) {
+	public void setNotificationMapper(@Nullable OutboundMessageMapper<Notification> notificationMapper) {
 		this.notificationMapper = notificationMapper;
 	}
 
@@ -108,8 +110,9 @@ public String getComponentType() {
 	@Override
 	public final void onInit() {
 		Assert.isTrue(this.getBeanFactory() instanceof ListableBeanFactory, "A ListableBeanFactory is required.");
-		Map<String, MBeanExporter> exporters = BeanFactoryUtils.beansOfTypeIncludingAncestors(
-				(ListableBeanFactory) this.getBeanFactory(), MBeanExporter.class);
+		Map<String, MBeanExporter> exporters =
+				BeanFactoryUtils.beansOfTypeIncludingAncestors((ListableBeanFactory) getBeanFactory(),
+						MBeanExporter.class);
 		Assert.isTrue(exporters.size() > 0, "No MBeanExporter is available in the current context.");
 		MBeanExporter exporter = null;
 		for (MBeanExporter exp : exporters.values()) {
@@ -128,7 +131,7 @@ public final void onInit() {
 	}
 
 	@Override
-	protected void handleMessageInternal(Message<?> message) throws Exception {
+	protected void handleMessageInternal(Message<?> message) throws Exception { // NOSONAR
 		this.delegate.publish(this.notificationMapper.fromMessage(message));
 	}
 
@@ -141,7 +144,7 @@ protected void handleMessageInternal(Message<?> message) throws Exception {
 	@IntegrationManagedResource
 	public static class PublisherDelegate implements NotificationPublisherAware {
 
-		private volatile NotificationPublisher notificationPublisher;
+		private NotificationPublisher notificationPublisher;
 
 		@Override
 		public void setNotificationPublisher(NotificationPublisher notificationPublisher) {