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>

8 files changed, 57 insertions, 32 deletions

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<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 @@ 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<Events> listeners =
+    private static final Queue<Events> 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<Server> {
         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<ServerPoolApi> impl =
+    public static final OrderedServiceImpl<ServerPoolApi> 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<Class<?>> servletClasses() {
+    public Collection<Class<?>> 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<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,