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 <jrh3@att.com>
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 3284a6b..a1afebc 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.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 @@
// 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 @@
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<Server> getBackups() {
+ /*
+ * For some reason, the junit tests break if Set.of() is used, so we'll stick with
+ * the long way for now.
+ */
+ Set<Server> 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 @@
* 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 176d39a..d2ea1a5 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 @@
* 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<Events> listeners =
+ private static final Queue<Events> listeners =
new ConcurrentLinkedQueue<>();
/**
@@ -64,7 +64,8 @@
*
* @param server this is the new server
*/
- public default void newServer(Server server) {
+ public void newServer(Server server) {
+ // do nothing
}
/**
@@ -72,7 +73,8 @@
*
* @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 @@
*
* @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 @@
*
* @param server the lead server that failed
*/
- public default void leaderFailed(Server server) {
+ public void leaderFailed(Server server) {
+ // do nothing
}
/**
@@ -98,6 +102,7 @@
*
* @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 5f93d2a..064af79 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 @@
* {@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 1a59645..be29170 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 @@
* 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 ffddf0a..61a950a 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 @@
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 0442912..8ec7c10 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 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<ServerPoolApi> impl =
+ public static final OrderedServiceImpl<ServerPoolApi> impl =
new OrderedServiceImpl<>(ServerPoolApi.class);
/**
@@ -39,7 +39,7 @@
*
* @return a Collection of classes implementing REST methods
*/
- public default Collection<Class<?>> servletClasses() {
+ public Collection<Class<?>> servletClasses() {
return Collections.emptyList();
}
@@ -50,7 +50,8 @@
*
* @param bucket the bucket that needs restoring
*/
- public default void restoreBucket(Bucket bucket) {
+ public void restoreBucket(Bucket bucket) {
+ // do nothing
}
/**
@@ -60,7 +61,8 @@
* @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 @@
* @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 470801e..f46013c 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 @@
* 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 c1d7192..18afcd4 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 @@
* 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 @@
MediaType.APPLICATION_OCTET_STREAM_TYPE);
final int count = lockCount;
- // build list of backup servers
- Set<Server> 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<String> entity, final int count,