From 5c2ca039b4e57adcb0b6bf784aa0acbabf434682 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Fri, 18 Sep 2020 11:48:06 -0400 Subject: Fix more sonars in feature-server-pool Addressed the following sonars: - move fields from "interface" to "class" - don't always return the same value - remove commented-out code - make constants "final" - don't synchronize on parameters Issue-ID: POLICY-2546 Change-Id: If2d410664d956a7efabf3a4dbef96bbf7d24de5e Signed-off-by: Jim Hahn --- .../org/onap/policy/drools/serverpool/Bucket.java | 34 ++++++++++++++++++---- .../org/onap/policy/drools/serverpool/Events.java | 19 +++++++----- .../drools/serverpool/FeatureServerPool.java | 5 ++-- .../org/onap/policy/drools/serverpool/Leader.java | 2 +- .../org/onap/policy/drools/serverpool/Server.java | 2 +- .../policy/drools/serverpool/ServerPoolApi.java | 15 ++++++---- .../onap/policy/drools/serverpool/TargetLock.java | 2 +- .../drools/serverpool/persistence/Persistence.java | 10 ++----- 8 files changed, 57 insertions(+), 32 deletions(-) (limited to 'feature-server-pool/src') diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java index 3284a6b7..a1afebc9 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java @@ -45,12 +45,14 @@ import java.util.Base64; import java.util.Comparator; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Queue; import java.util.Random; +import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; import java.util.UUID; @@ -109,7 +111,9 @@ public class Bucket { // create MD5 MessageDigest -- used to hash keywords try { - messageDigest = MessageDigest.getInstance("MD5"); + // disabling sonar, as the digest is only used to hash keywords - it isn't + // used for security purposes + messageDigest = MessageDigest.getInstance("MD5"); // NOSONAR } catch (NoSuchAlgorithmException e) { throw new ExceptionInInitializerError(e); } @@ -406,14 +410,32 @@ public class Bucket { logger.info("Bucket {} owner: {}->null", index, bucket.getOwner()); bucketChanges = true; - synchronized (bucket) { - bucket.setOwner(null); - bucket.setState(null); - } + bucket.nullifyOwner(); } return bucketChanges; } + private synchronized void nullifyOwner() { + setOwner(null); + setState(null); + } + + /** + * Gets the set of backups. + * + * @return the set of backups + */ + public synchronized Set getBackups() { + /* + * For some reason, the junit tests break if Set.of() is used, so we'll stick with + * the long way for now. + */ + Set backups = new HashSet<>(); + backups.add(getPrimaryBackup()); + backups.add(getSecondaryBackup()); + return backups; + } + private static boolean updatePrimaryBackup(DataInputStream dis, int index, Bucket bucket, boolean bucketChanges) throws IOException { Server newPrimaryBackup = @@ -1017,7 +1039,7 @@ public class Bucket { * is registered to listen for notifications of state transitions. Note * that all of these methods are running within the 'MainLoop' thread. */ - private static class EventHandler implements Events { + private static class EventHandler extends Events { /** * {@inheritDoc} */ diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Events.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Events.java index 176d39ac..d2ea1a5c 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Events.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Events.java @@ -28,9 +28,9 @@ import java.util.concurrent.ConcurrentLinkedQueue; * This interface is used to distribute notifications of various system * events, such as new 'Server' instances, or a server failing. */ -public interface Events { +public class Events { // set of listeners receiving event notifications - static final Queue listeners = + private static final Queue listeners = new ConcurrentLinkedQueue<>(); /** @@ -64,7 +64,8 @@ public interface Events { * * @param server this is the new server */ - public default void newServer(Server server) { + public void newServer(Server server) { + // do nothing } /** @@ -72,7 +73,8 @@ public interface Events { * * @param server this is the server that failed */ - public default void serverFailed(Server server) { + public void serverFailed(Server server) { + // do nothing } /** @@ -80,7 +82,8 @@ public interface Events { * * @param server this is the new lead server */ - public default void newLeader(Server server) { + public void newLeader(Server server) { + // do nothing } /** @@ -88,7 +91,8 @@ public interface Events { * * @param server the lead server that failed */ - public default void leaderFailed(Server server) { + public void leaderFailed(Server server) { + // do nothing } /** @@ -98,6 +102,7 @@ public interface Events { * * @param server the current leader, which has been confirmed */ - public default void leaderConfirmed(Server server) { + public void leaderConfirmed(Server server) { + // do nothing } } diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/FeatureServerPool.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/FeatureServerPool.java index 5f93d2a3..064af79e 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/FeatureServerPool.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/FeatureServerPool.java @@ -205,8 +205,9 @@ public class FeatureServerPool * {@inheritDoc} */ @Override - public boolean insertDrools( - final PolicySession session, final Object object) { + public boolean insertDrools(final PolicySession session, final Object object) { // NOSONAR + // sonar complained that the method always returns the same value. However, + // we prefer the code be structured this way, thus disabled sonar final String keyword = Keyword.lookupKeyword(object); if (keyword == null) { diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Leader.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Leader.java index 1a596455..be291708 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Leader.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Leader.java @@ -157,7 +157,7 @@ class Leader { * is registered to listen for notifications of state transitions. Note * that all of these methods are running within the 'MainLoop' thread. */ - private static class EventHandler implements Events { + private static class EventHandler extends Events { /** * {@inheritDoc} */ diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java index ffddf0a0..61a950ae 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java @@ -246,7 +246,7 @@ public class Server implements Comparable { Properties prop = new Properties(); for (String arg : args) { - // arguments with an equals sign in them are a property definition; + // arguments with an equals sign in them are a property definition - // otherwise, they are a properties file name if (arg.contains("=")) { diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java index 0442912f..8ec7c100 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java @@ -25,12 +25,12 @@ import java.util.Collections; import org.onap.policy.common.utils.services.OrderedService; import org.onap.policy.common.utils.services.OrderedServiceImpl; -public interface ServerPoolApi extends OrderedService { +public abstract class ServerPoolApi implements OrderedService { /** * 'ServerPoolApi.impl.getList()' returns an ordered list of objects * implementing the 'ServerPoolApi' interface. */ - public static OrderedServiceImpl impl = + public static final OrderedServiceImpl impl = new OrderedServiceImpl<>(ServerPoolApi.class); /** @@ -39,7 +39,7 @@ public interface ServerPoolApi extends OrderedService { * * @return a Collection of classes implementing REST methods */ - public default Collection> servletClasses() { + public Collection> servletClasses() { return Collections.emptyList(); } @@ -50,7 +50,8 @@ public interface ServerPoolApi extends OrderedService { * * @param bucket the bucket that needs restoring */ - public default void restoreBucket(Bucket bucket) { + public void restoreBucket(Bucket bucket) { + // do nothing } /** @@ -60,7 +61,8 @@ public interface ServerPoolApi extends OrderedService { * @param bucket the bucket containing the 'GlobalLocks' adjunct * @param globalLocks the 'GlobalLocks' adjunct */ - public default void lockUpdate(Bucket bucket, TargetLock.GlobalLocks globalLocks) { + public void lockUpdate(Bucket bucket, TargetLock.GlobalLocks globalLocks) { + // do nothing } /** @@ -74,6 +76,7 @@ public interface ServerPoolApi extends OrderedService { * @param isOwner 'true' if the current host owns the bucket * @param isBackup 'true' if the current host is a backup for the bucket */ - public default void auditBucket(Bucket bucket, boolean isOwner, boolean isBackup) { + public void auditBucket(Bucket bucket, boolean isOwner, boolean isBackup) { + // do nothing } } diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/TargetLock.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/TargetLock.java index 470801e2..f46013ca 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/TargetLock.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/TargetLock.java @@ -821,7 +821,7 @@ public class TargetLock implements Lock, Serializable { * There is a single instance of class 'TargetLock.EventHandler', which is * registered to listen for notifications of state transitions. */ - private static class EventHandler implements Events { + private static class EventHandler extends Events { /** * {@inheritDoc} */ diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java index c1d7192f..18afcd47 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java @@ -68,7 +68,7 @@ import org.slf4j.LoggerFactory; * backing up the data of selected Drools sessions and server-side 'TargetLock' * data on separate hosts. */ -public class Persistence implements PolicySessionFeatureApi, ServerPoolApi { +public class Persistence extends ServerPoolApi implements PolicySessionFeatureApi { private static Logger logger = LoggerFactory.getLogger(Persistence.class); // HTTP query parameters @@ -240,13 +240,7 @@ public class Persistence implements PolicySessionFeatureApi, ServerPoolApi { MediaType.APPLICATION_OCTET_STREAM_TYPE); final int count = lockCount; - // build list of backup servers - Set servers = new HashSet<>(); - synchronized (bucket) { - servers.add(bucket.getPrimaryBackup()); - servers.add(bucket.getSecondaryBackup()); - } - sendLocksToBackupServers(bucketNumber, entity, count, servers); + sendLocksToBackupServers(bucketNumber, entity, count, bucket.getBackups()); } private static void sendLocksToBackupServers(final int bucketNumber, final Entity entity, final int count, -- cgit 1.2.3-korg