Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -145,7 +155,7 @@ public int getPhase() {
}

@SuppressWarnings("unchecked")
protected final S _this() {
protected final S _this() { // NOSONAR
return (S) this;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -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();
Expand All @@ -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
Expand All @@ -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;
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -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) {
Expand All @@ -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;
}

Expand All @@ -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()) {
Expand All @@ -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));
}

Expand All @@ -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) {
Expand Down
Loading