From e70fd0d2278f2acc75cedc98d411488c3ecd9b75 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Sun, 26 Jan 2020 10:38:27 -0500 Subject: [PATCH] Fix new Sonar smells --- .../router/AbstractMappingMessageRouter.java | 8 +- .../leader/LockRegistryLeaderInitiator.java | 100 ++++++++++-------- .../transformer/SyslogToMapTransformer.java | 14 +-- 3 files changed, 65 insertions(+), 57 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMappingMessageRouter.java b/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMappingMessageRouter.java index 0655366669..e43b10dccb 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMappingMessageRouter.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMappingMessageRouter.java @@ -293,7 +293,9 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter return; } for (Object channelKey : channelKeys) { - addChannelKeyToCollection(channels, message, channelKey); + if (channelKey != null) { + addChannelKeyToCollection(channels, message, channelKey); + } } } @@ -319,13 +321,13 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter else if (channelKey instanceof Collection) { addToCollection(channels, (Collection) channelKey, message); } - else if (channelKey != null && conversionService.canConvert(channelKey.getClass(), String.class)) { + else if (conversionService.canConvert(channelKey.getClass(), String.class)) { String converted = conversionService.convert(channelKey, String.class); if (converted != null) { addChannelFromString(channels, converted, message); } } - else if (channelKey != null) { + else { throw new MessagingException("unsupported return type for router [" + channelKey.getClass() + "]"); } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java b/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java index 4fb49b933d..8114b3d64c 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java @@ -353,42 +353,7 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe try { while (isRunning()) { try { - - if (logger.isDebugEnabled()) { - logger.debug("Acquiring the lock for " + this.context); - } - - // We always try to acquire the lock, in case it expired - boolean acquired = this.lock.tryLock(LockRegistryLeaderInitiator.this.heartBeatMillis, - TimeUnit.MILLISECONDS); - if (!this.locked) { - if (acquired) { - // Success: we are now leader - this.locked = true; - handleGranted(); - } - else if (isPublishFailedEvents()) { - publishFailedToAcquire(); - } - } - else if (acquired) { - // If we were able to acquire it but we were already locked we - // should release it - this.lock.unlock(); - if (isRunning()) { - // Give it a chance to expire. - Thread.sleep(LockRegistryLeaderInitiator.this.heartBeatMillis); - } - } - else { - this.locked = false; - // We were not able to acquire it, therefore not leading any more - handleRevoked(); - if (isRunning()) { - // Try again quickly in case the lock holder dropped it - Thread.sleep(LockRegistryLeaderInitiator.this.busyWaitMillis); - } - } + tryAcquireLock(); } catch (Exception e) { if (handleLockException(e)) { @@ -414,7 +379,44 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe return null; } - private boolean handleLockException(Exception e) { + private void tryAcquireLock() throws InterruptedException { + if (logger.isDebugEnabled()) { + logger.debug("Acquiring the lock for " + this.context); + } + // We always try to acquire the lock, in case it expired + boolean acquired = this.lock.tryLock(LockRegistryLeaderInitiator.this.heartBeatMillis, + TimeUnit.MILLISECONDS); + if (!this.locked) { + if (acquired) { + // Success: we are now leader + this.locked = true; + handleGranted(); + } + else if (isPublishFailedEvents()) { + publishFailedToAcquire(); + } + } + else if (acquired) { + // If we were able to acquire it but we were already locked we + // should release it + this.lock.unlock(); + if (isRunning()) { + // Give it a chance to expire. + Thread.sleep(LockRegistryLeaderInitiator.this.heartBeatMillis); + } + } + else { + this.locked = false; + // We were not able to acquire it, therefore not leading any more + handleRevoked(); + if (isRunning()) { + // Try again quickly in case the lock holder dropped it + Thread.sleep(LockRegistryLeaderInitiator.this.busyWaitMillis); + } + } + } + + private boolean handleLockException(Exception ex) { if (this.locked) { this.locked = false; try { @@ -429,17 +431,10 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe handleRevoked(); } - if (e instanceof InterruptedException || Thread.currentThread().isInterrupted()) { + if (ex instanceof InterruptedException || Thread.currentThread().isInterrupted()) { Thread.currentThread().interrupt(); if (isRunning()) { - logger.warn("Restarting LeaderSelector for " + this.context + " because of error.", e); - LockRegistryLeaderInitiator.this.future = - LockRegistryLeaderInitiator.this.executorService.submit( - () -> { - // Give it a chance to elect some other leader. - Thread.sleep(LockRegistryLeaderInitiator.this.busyWaitMillis); - return call(); - }); + restartSelectorBecauseOfError(ex); } return true; } @@ -456,12 +451,23 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe } if (logger.isDebugEnabled()) { logger.debug("Error acquiring the lock for " + this.context + - ". " + (isRunning() ? "Retrying..." : ""), e); + ". " + (isRunning() ? "Retrying..." : ""), ex); } } return false; } + private void restartSelectorBecauseOfError(Exception ex) { + logger.warn("Restarting LeaderSelector for " + this.context + " because of error.", ex); + LockRegistryLeaderInitiator.this.future = + LockRegistryLeaderInitiator.this.executorService.submit( + () -> { + // Give it a chance to elect some other leader. + Thread.sleep(LockRegistryLeaderInitiator.this.busyWaitMillis); + return call(); + }); + } + public boolean isLeader() { return this.locked; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/transformer/SyslogToMapTransformer.java b/spring-integration-core/src/main/java/org/springframework/integration/transformer/SyslogToMapTransformer.java index 0153475464..c6462849bb 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/transformer/SyslogToMapTransformer.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/transformer/SyslogToMapTransformer.java @@ -96,20 +96,20 @@ public class SyslogToMapTransformer extends AbstractPayloadTransformer map) { try { - String facilityString = matcher.group(1); + String facilityString = matcher.group(1); // NOSONAR int facility = Integer.parseInt(facilityString); int severity = facility & 0x7; facility = facility >> 3; map.put(FACILITY, facility); map.put(SEVERITY, severity); - String timestamp = matcher.group(2); + String timestamp = matcher.group(2); // NOSONAR parseTimestampToMap(timestamp, map); - map.put(HOST, matcher.group(3)); - String tag = matcher.group(4); + map.put(HOST, matcher.group(3)); // NOSONAR + String tag = matcher.group(4); // NOSONAR if (StringUtils.hasLength(tag)) { map.put(TAG, tag); } - map.put(MESSAGE, matcher.group(5)); + map.put(MESSAGE, matcher.group(5)); // NOSONAR } catch (Exception e) { if (logger.isDebugEnabled()) { @@ -132,10 +132,10 @@ public class SyslogToMapTransformer extends AbstractPayloadTransformer