Sonar fixes

- printStackTrace in test mail server
- Illegal throws
- Large anon. classes

* - indexOf char
- stored external object

* - ignored exceptional return values

* - checkstyle
This commit is contained in:
Gary Russell
2019-04-30 13:57:13 -04:00
committed by Artem Bilan
parent 4199ff57cd
commit 0bb901b286
19 changed files with 280 additions and 235 deletions

View File

@@ -47,8 +47,8 @@ public class MessageGroupExpiredEvent extends IntegrationEvent {
super(source);
this.groupId = groupId;
this.messageCount = messageCount;
this.lastModified = lastModified;
this.expired = expired;
this.lastModified = (Date) lastModified.clone();
this.expired = (Date) expired.clone();
this.discarded = discarded;
}

View File

@@ -163,7 +163,7 @@ public class MessagingGatewayRegistrar implements ImportBeanDefinitionRegistrar
}
String id = (String) gatewayAttributes.get("name");
if (!StringUtils.hasText(id)) {
id = Introspector.decapitalize(serviceInterface.substring(serviceInterface.lastIndexOf(".") + 1));
id = Introspector.decapitalize(serviceInterface.substring(serviceInterface.lastIndexOf('.') + 1));
}
gatewayProxyBuilder.addConstructorArgValue(serviceInterface);

View File

@@ -35,42 +35,49 @@ import org.springframework.util.xml.DomUtils;
*
* @author Mark Fisher
* @author Artem Bilan
* @author Gary Russell
*/
public class AnnotationConfigParser implements BeanDefinitionParser {
@Override
public BeanDefinition parse(final Element element, ParserContext parserContext) {
StandardAnnotationMetadata importingClassMetadata =
new StandardAnnotationMetadata(Object.class) {
@Override
public Map<String, Object> getAnnotationAttributes(String annotationType) {
if (EnablePublisher.class.getName().equals(annotationType)) {
Element enablePublisherElement =
DomUtils.getChildElementByTagName(element, "enable-publisher");
if (enablePublisherElement != null) {
Map<String, Object> attributes = new HashMap<>();
attributes.put("defaultChannel",
enablePublisherElement.getAttribute("default-publisher-channel"));
attributes.put("proxyTargetClass",
enablePublisherElement.getAttribute("proxy-target-class"));
attributes.put("order", enablePublisherElement.getAttribute("order"));
return attributes;
}
else {
return null;
}
}
else {
return null;
}
}
};
new IntegrationRegistrar().registerBeanDefinitions(importingClassMetadata, parserContext.getRegistry());
new IntegrationRegistrar().registerBeanDefinitions(new ExtendedAnnotationMetadata(Object.class, element),
parserContext.getRegistry());
return null;
}
private static final class ExtendedAnnotationMetadata extends StandardAnnotationMetadata {
private final Element element;
ExtendedAnnotationMetadata(Class<?> introspectedClass, Element element) {
super(introspectedClass);
this.element = element;
}
@Override
public Map<String, Object> getAnnotationAttributes(String annotationType) {
if (EnablePublisher.class.getName().equals(annotationType)) {
Element enablePublisherElement =
DomUtils.getChildElementByTagName(this.element, "enable-publisher");
if (enablePublisherElement != null) {
Map<String, Object> attributes = new HashMap<>();
attributes.put("defaultChannel",
enablePublisherElement.getAttribute("default-publisher-channel"));
attributes.put("proxyTargetClass",
enablePublisherElement.getAttribute("proxy-target-class"));
attributes.put("order", enablePublisherElement.getAttribute("order"));
return attributes;
}
else {
return null;
}
}
else {
return null;
}
}
}
}

View File

@@ -132,38 +132,7 @@ public class ReactiveStreamsConsumer extends AbstractEndpoint implements Integra
if (this.lifecycleDelegate != null) {
this.lifecycleDelegate.start();
}
this.publisher.subscribe(new BaseSubscriber<Message<?>>() {
private final Subscriber<Message<?>> delegate = ReactiveStreamsConsumer.this.subscriber;
@Override
public void hookOnSubscribe(Subscription s) {
this.delegate.onSubscribe(s);
ReactiveStreamsConsumer.this.subscription = s;
}
@Override
public void hookOnNext(Message<?> message) {
try {
this.delegate.onNext(message);
}
catch (Exception e) {
ReactiveStreamsConsumer.this.errorHandler.handleError(e);
hookOnError(e);
}
}
@Override
public void hookOnError(Throwable t) {
this.delegate.onError(t);
}
@Override
public void hookOnComplete() {
this.delegate.onComplete();
}
});
this.publisher.subscribe(new DelegatingSubscriber());
}
@Override
@@ -176,6 +145,42 @@ public class ReactiveStreamsConsumer extends AbstractEndpoint implements Integra
}
}
private final class DelegatingSubscriber extends BaseSubscriber<Message<?>> {
private final Subscriber<Message<?>> delegate = ReactiveStreamsConsumer.this.subscriber;
DelegatingSubscriber() {
super();
}
@Override
public void hookOnSubscribe(Subscription s) {
this.delegate.onSubscribe(s);
ReactiveStreamsConsumer.this.subscription = s;
}
@Override
public void hookOnNext(Message<?> message) {
try {
this.delegate.onNext(message);
}
catch (Exception e) {
ReactiveStreamsConsumer.this.errorHandler.handleError(e);
hookOnError(e);
}
}
@Override
public void hookOnError(Throwable t) {
this.delegate.onError(t);
}
@Override
public void hookOnComplete() {
this.delegate.onComplete();
}
}
private static final class MessageHandlerSubscriber
implements CoreSubscriber<Message<?>>, Disposable, Lifecycle {

View File

@@ -465,7 +465,7 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint
}
@Nullable
protected Object doInvoke(MethodInvocation invocation, boolean runningOnCallerThread) throws Throwable {
protected Object doInvoke(MethodInvocation invocation, boolean runningOnCallerThread) throws Throwable { // NOSONAR
Method method = invocation.getMethod();
if (AopUtils.isToStringMethod(method)) {
return "gateway proxy for service interface [" + this.serviceInterface + "]";
@@ -474,7 +474,7 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint
return invokeGatewayMethod(invocation, runningOnCallerThread);
}
catch (Throwable e) { //NOSONAR - ok to catch, rethrown below
this.rethrowExceptionCauseIfPossible(e, invocation.getMethod());
rethrowExceptionCauseIfPossible(e, invocation.getMethod());
return null; // preceding call should always throw something
}
}
@@ -538,7 +538,7 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint
return (response != null) ? this.convert(response, returnType) : null;
}
private void rethrowExceptionCauseIfPossible(Throwable originalException, Method method) throws Throwable {
private void rethrowExceptionCauseIfPossible(Throwable originalException, Method method) throws Throwable { // NOSONAR
Class<?>[] exceptionTypes = method.getExceptionTypes();
Throwable t = originalException;
while (t != null) {

View File

@@ -19,8 +19,6 @@ package org.springframework.integration.handler.advice;
import java.lang.reflect.Method;
import org.aopalliance.intercept.MethodInvocation;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.integration.context.IntegrationObjectSupport;
import org.springframework.messaging.Message;
@@ -37,10 +35,8 @@ import org.springframework.messaging.MessageHandler;
*/
public abstract class AbstractHandleMessageAdvice extends IntegrationObjectSupport implements HandleMessageAdvice {
protected final Log logger = LogFactory.getLog(this.getClass());
@Override
public final Object invoke(MethodInvocation invocation) throws Throwable {
public final Object invoke(MethodInvocation invocation) throws Throwable { // NOSONAR
Method method = invocation.getMethod();
Object invocationThis = invocation.getThis();
Object[] arguments = invocation.getArguments();

View File

@@ -64,42 +64,7 @@ public abstract class AbstractRequestHandlerAdvice extends IntegrationObjectSupp
else {
Message<?> message = (Message<?>) arguments[0];
try {
return doInvoke(new ExecutionCallback() {
@Override
public Object execute() {
try {
return invocation.proceed();
}
catch (Throwable e) { //NOSONAR - ok to catch; unwrapped and rethrown below
throw new ThrowableHolderException(e);
}
}
@Override
public Object cloneAndExecute() {
try {
/*
* If we don't copy the invocation carefully it won't keep a reference to the other
* interceptors in the chain.
*/
if (invocation instanceof ProxyMethodInvocation) {
return ((ProxyMethodInvocation) invocation).invocableClone().proceed();
}
else {
throw new IllegalStateException(
"MethodInvocation of the wrong type detected - this should not happen with Spring AOP," +
" so please raise an issue if you see this exception");
}
}
catch (Exception e) { //NOSONAR - catch necessary so we can wrap Errors
throw new MessagingException(message, "Failed to handle", e);
}
catch (Throwable e) { //NOSONAR - ok to catch; unwrapped and rethrown below
throw new ThrowableHolderException(e);
}
}
}, invocationThis, message);
return doInvoke(new CallbackImpl(invocation), invocationThis, message);
}
catch (Exception e) {
throw this.unwrapThrowableIfNecessary(e);
@@ -172,6 +137,50 @@ public abstract class AbstractRequestHandlerAdvice extends IntegrationObjectSupp
}
private static final class CallbackImpl implements ExecutionCallback {
private final MethodInvocation invocation;
CallbackImpl(MethodInvocation invocation) {
this.invocation = invocation;
}
@Override
public Object execute() {
try {
return this.invocation.proceed();
}
catch (Throwable e) { //NOSONAR - ok to catch; unwrapped and rethrown below
throw new ThrowableHolderException(e);
}
}
@Override
public Object cloneAndExecute() {
try {
/*
* If we don't copy the invocation carefully it won't keep a reference to the other
* interceptors in the chain.
*/
if (this.invocation instanceof ProxyMethodInvocation) {
return ((ProxyMethodInvocation) this.invocation).invocableClone().proceed();
}
else {
throw new IllegalStateException(
"MethodInvocation of the wrong type detected - this should not happen with Spring AOP," +
" so please raise an issue if you see this exception");
}
}
catch (Exception e) { //NOSONAR - catch necessary so we can wrap Errors
throw new MessagingException((Message<?>) this.invocation.getArguments()[0], "Failed to handle", e);
}
catch (Throwable e) { //NOSONAR - ok to catch; unwrapped and rethrown below
throw new ThrowableHolderException(e);
}
}
}
@SuppressWarnings("serial")
protected static final class ThrowableHolderException extends RuntimeException {

View File

@@ -93,11 +93,13 @@ public class PropertiesPersistingMetadataStore implements ConcurrentMetadataStor
@Override
public void afterPropertiesSet() {
File baseDir = new File(this.baseDirectory);
baseDir.mkdirs();
if (!baseDir.mkdirs() && this.logger.isWarnEnabled()) {
this.logger.warn("Failed to create directories for " + baseDir);
}
this.file = new File(baseDir, this.fileName);
try {
if (!this.file.exists()) {
this.file.createNewFile();
if (!this.file.exists() && !this.file.createNewFile() && this.logger.isWarnEnabled()) {
this.logger.warn("Failed to create file " + this.file);
}
}
catch (Exception e) {

View File

@@ -21,44 +21,59 @@ import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
/**
* The {@link LockRegistry} implementation which has no effect. Mainly used in cases where locking itself must be conditional
* but an extra IF statement would clutter the code.
* For example. In the FILE module FileWritingMessageHandler is initialized with this instance of LockRegistry by default
* since real locking is only required if its 'append' flag is set to true.
* The {@link LockRegistry} implementation which has no effect. Mainly used in cases where
* locking itself must be conditional but an extra IF statement would clutter the code.
* For example. In the FILE module FileWritingMessageHandler is initialized with this
* instance of LockRegistry by default since real locking is only required if its 'append'
* flag is set to true.
*
* @author Oleg Zhurakousky
* @author Gary Russell
* @since 2.2
*
*/
public final class PassThruLockRegistry implements LockRegistry {
@Override
public Lock obtain(Object lockKey) {
return new Lock() {
public void unlock() {
// noop
}
public boolean tryLock(long time, TimeUnit unit)
throws InterruptedException {
return true;
}
public boolean tryLock() {
return true;
}
public Condition newCondition() {
throw new UnsupportedOperationException("This method is not supported for this implementation of Lock");
}
public void lockInterruptibly() throws InterruptedException {
// noop
}
public void lock() {
// noop
}
};
return new PassThruLock();
}
private static final class PassThruLock implements Lock {
PassThruLock() {
super();
}
@Override
public void unlock() {
// noop
}
@Override
public boolean tryLock(long time, TimeUnit unit) {
return true;
}
@Override
public boolean tryLock() {
return true;
}
@Override
public Condition newCondition() {
throw new UnsupportedOperationException("This method is not supported for this implementation of Lock");
}
@Override
public void lockInterruptibly() {
// noop
}
@Override
public void lock() {
// noop
}
}
}