Sonar cleanup for PolicyEngineUtils

Changes to resolve some sonar "major" issues in the PolicyEngineUtils
module.

Patch 2: Updated the NotificationStoreTest junit to account for the return
of a "Collections.emptyList()" rather than "null" in StdPDPNotification
class.

Patch 3: Updated the StdPDPNotificationTest junit to account for the
return of a "Collections.emptyList()" rather than "null" in
StdPDPNotifaction class.

Issue-ID: POLICY-474
Change-Id: I925774ac181fd38d1206637f47ab27ba499629ef
Signed-off-by: Magnusen, Drew (dm741q) <dm741q@att.com>

13 files changed, 60 insertions, 33 deletions

diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/std/NotificationStore.java b/PolicyEngineUtils/src/main/java/org/onap/policy/std/NotificationStore.java
index 8c6c9580d..47b23605d 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/std/NotificationStore.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/std/NotificationStore.java
@@ -40,6 +40,11 @@ public class NotificationStore {
 
     private static StdPDPNotification notificationRecord = new StdPDPNotification();
 
+    public NotificationStore () {
+    	// Sonar prefers that we have an empty public constructor
+    	// as opposed to an implicit public constructor.
+    }
+    
     public static StdPDPNotification getDeltaNotification(StdPDPNotification newNotification) {
         StdPDPNotification notificationDelta = new StdPDPNotification();
         ArrayList<StdRemovedPolicy> removedDelta = new ArrayList<>();
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdLoadedPolicy.java b/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdLoadedPolicy.java
index 10268f658..ebbed2100 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdLoadedPolicy.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdLoadedPolicy.java
@@ -34,7 +34,7 @@ public class StdLoadedPolicy implements LoadedPolicy{
 	@Override
 	public String getPolicyName() {
 		if(policyName!=null && policyName.contains(".xml")){
-			return (policyName.substring(0, policyName.substring(0, policyName.lastIndexOf('.')).lastIndexOf('.')));
+			return policyName.substring(0, policyName.substring(0, policyName.lastIndexOf('.')).lastIndexOf('.'));
 		}
 		return this.policyName;
 	}
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdPDPNotification.java b/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdPDPNotification.java
index 1640fe8db..54b9bb0f6 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdPDPNotification.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdPDPNotification.java
@@ -21,6 +21,7 @@
 package org.onap.policy.std;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import org.onap.policy.api.LoadedPolicy;
 import org.onap.policy.api.NotificationType;
@@ -42,7 +43,7 @@ public class StdPDPNotification implements PDPNotification {
             removed.addAll(removedPolicies);
             return this.removed;
         }
-        return null;
+        return Collections.emptyList();
     }
 
     @Override
@@ -52,7 +53,7 @@ public class StdPDPNotification implements PDPNotification {
             updated.addAll(loadedPolicies);
             return updated;
         }
-        return null;
+        return Collections.emptyList();
     }
 
     @Override
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdRemovedPolicy.java b/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdRemovedPolicy.java
index 5430e24ef..04ec6981b 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdRemovedPolicy.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/std/StdRemovedPolicy.java
@@ -38,7 +38,7 @@ public class StdRemovedPolicy implements RemovedPolicy{
 	@Override
 	public String getPolicyName() {
 		if(policyName!=null && policyName.contains(".xml")){
-			return (policyName.substring(0, policyName.substring(0, policyName.lastIndexOf('.')).lastIndexOf('.')));
+			return policyName.substring(0, policyName.substring(0, policyName.lastIndexOf('.')).lastIndexOf('.'));
 		}
 		return this.policyName;
 	}
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyClientImpl.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyClientImpl.java
index ecb51b286..9d5222af2 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyClientImpl.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyClientImpl.java
@@ -123,6 +123,7 @@ public class AAFPolicyClientImpl implements AAFPolicyClient{
 	 * @param properties  Properties with CLIENT_ID, CLIENT_KEY and ENVIRONMENT
 	 * @throws AAFPolicyException exceptions if any.
 	 */
+	@Override
 	public void updateProperties(Properties properties) throws AAFPolicyException{
 		setup(properties);
 	}
@@ -137,6 +138,7 @@ public class AAFPolicyClientImpl implements AAFPolicyClient{
 	 * @param action Permissions Action. 
 	 * @return
 	 */
+	@Override
 	public boolean checkAuthPerm(String mechID, String pass, String type, String instance, String action){
 		return checkAuth(mechID, pass) && checkPerm(mechID, pass, type, instance, action);
 	}
@@ -148,6 +150,7 @@ public class AAFPolicyClientImpl implements AAFPolicyClient{
 	 * @param pass Password.
 	 * @return True or False. 
 	 */
+	@Override
 	public boolean checkAuth(String userName, String pass){
 		if(aafAuthn!=null){
 			try {
@@ -176,6 +179,7 @@ public class AAFPolicyClientImpl implements AAFPolicyClient{
 	 * @param action Permissions Action. 
 	 * @return True or False. 
 	 */
+	@Override
 	public boolean checkPerm(String userName, String pass, String type, String instance, String action){
 		int i =0;
 		Boolean result= false;
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyException.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyException.java
index d476d2c95..b67b02243 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyException.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/AAFPolicyException.java
@@ -28,6 +28,8 @@ public class AAFPolicyException extends Exception {
 	private static final long serialVersionUID = 1910606668038621L;
 	
 	public AAFPolicyException() {
+		// Empty constructor.
+		// Nothing needs to be initialized in this exception class.
 	}
 	
 	public AAFPolicyException(String message) {
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpHandler.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpHandler.java
index f45ba56da..4dfc64c39 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpHandler.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpHandler.java
@@ -30,6 +30,7 @@ public interface BackUpHandler extends NotificationHandler{
 	 * 
 	 * @param notification <code>PDPNotification</code> of {@link org.onap.policy.api.PDPNotification} is the object that has information of the notification.
 	 */
+	@Override
 	public void notificationReceived(PDPNotification notification);
 	
 	/**
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitor.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitor.java
index 9aab2ad45..1353cc607 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitor.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitor.java
@@ -134,8 +134,8 @@ public class BackUpMonitor {
      */
     public static synchronized BackUpMonitor getInstance(String resourceNodeName, String resourceName,
             Properties properties, BackUpHandler handler) throws BackUpMonitorException {
-        if (resourceNodeName == null || resourceNodeName.trim().equals("") || resourceName == null
-                || resourceName.trim().equals("") || properties == null || handler == null) {
+        if (resourceNodeName == null || "".equals(resourceNodeName.trim()) || resourceName == null
+                || "".equals(resourceName.trim()) || properties == null || handler == null) {
             LOGGER.error("Error while getting Instance. Please check resourceName and/or properties file");
             return null;
         } else if ((resourceNodeName.equals(ResourceNode.ASTRA.toString())
@@ -149,22 +149,22 @@ public class BackUpMonitor {
     // This is to validate given Properties with required values.
     private static Boolean validate(Properties properties) {
         if (properties.getProperty("javax.persistence.jdbc.driver") == null
-                || properties.getProperty("javax.persistence.jdbc.driver").trim().equals("")) {
+                || "".equals(properties.getProperty("javax.persistence.jdbc.driver").trim())) {
             LOGGER.error("javax.persistence.jdbc.driver property is empty");
             return false;
         }
         if (properties.getProperty("javax.persistence.jdbc.url") == null
-                || properties.getProperty("javax.persistence.jdbc.url").trim().equals("")) {
+                || "".equals(properties.getProperty("javax.persistence.jdbc.url").trim())) {
             LOGGER.error("javax.persistence.jdbc.url property is empty");
             return false;
         }
         if (properties.getProperty("javax.persistence.jdbc.user") == null
-                || properties.getProperty("javax.persistence.jdbc.user").trim().equals("")) {
+                || "".equals(properties.getProperty("javax.persistence.jdbc.user").trim())) {
             LOGGER.error("javax.persistence.jdbc.user property is empty");
             return false;
         }
         if (properties.getProperty(PING_INTERVAL) == null
-                || properties.getProperty(PING_INTERVAL).trim().equals("")) {
+                || "".equals(properties.getProperty(PING_INTERVAL).trim())) {
             LOGGER.info("ping_interval property not specified. Taking default value");
         } else {
             try {
@@ -351,7 +351,7 @@ public class BackUpMonitor {
                             BackUpMonitorEntity masterEntity = masterEntities.get(0);
                             if (!masterEntity.getResourceName().equals(selfEntity.getResourceName())) {
                                 Date currentTime = new Date();
-                                long timeDiff = 0;
+                                long timeDiff;
                                 timeDiff = currentTime.getTime() - masterEntity.getTimeStamp().getTime();
                                 if (timeDiff > (pingInterval + 1500)) {
                                     // This is down or has an issue and we need to become Master while turning the
@@ -407,21 +407,25 @@ public class BackUpMonitor {
             JsonNode patchNode = JsonDiff.asJson(oldNotification, notification);
             LOGGER.info("Generated JSON Patch is " + patchNode.toString());
             JsonPatch patch = JsonPatch.fromJson(patchNode);
-            try {
-                JsonNode patched = patch.apply(oldNotification);
-                LOGGER.info("Generated New Notification is : " + patched.toString());
-                return patched.toString();
-            } catch (JsonPatchException e) {
-                LOGGER.error("Error generating Patched " + e.getMessage(), e);
-                return null;
-            }
+            return generatePatchNotification(patch, oldNotification);
         } catch (IOException e) {
             LOGGER.error("Error generating Patched " + e.getMessage(), e);
             return null;
         }
     }
 
-    /**
+    private String generatePatchNotification(JsonPatch patch, JsonNode oldNotification) {
+        try {
+            JsonNode patched = patch.apply(oldNotification);
+            LOGGER.info("Generated New Notification is : " + patched.toString());
+            return patched.toString();
+        } catch (JsonPatchException e) {
+            LOGGER.error("Error generating Patched " + e.getMessage(), e);
+            return null;
+        }
+	}
+
+	/**
      * Updates Notification in the Database while Performing the health check.
      * 
      * @param notification
@@ -440,12 +444,8 @@ public class BackUpMonitor {
                         StdPDPNotification.class);
                 if (notificationObject.getNotificationType() != null) {
                     LOGGER.info("Performing Patched notification ");
-                    try {
-                        handler.runOnNotification(notificationObject);
-                        notificationRecord = lastMasterNotification;
-                    } catch (Exception e) {
-                        LOGGER.error("Error in Clients Handler Object : " + e.getMessage(), e);
-                    }
+                    performPatchNotification(notificationObject);
+
                 }
             } catch (IOException e) {
                 LOGGER.info("Error while notification Conversion " + e.getMessage(), e);
@@ -453,11 +453,21 @@ public class BackUpMonitor {
         }
     }
 
-    // Used to set LastMasterNotification Record.
+    private static void performPatchNotification(PDPNotification notificationObject) {
+        try {
+            handler.runOnNotification(notificationObject);
+            notificationRecord = lastMasterNotification;
+        } catch (Exception e) {
+            LOGGER.error("Error in Clients Handler Object : " + e.getMessage(), e);
+        }
+		
+	}
+
+	// Used to set LastMasterNotification Record.
     private static void setLastNotification(String notification) {
         synchronized (notificationLock) {
             lastMasterNotification = notification;
-            if (lastMasterNotification != null && !lastMasterNotification.equals("\"notificationType\":null")) {
+            if (lastMasterNotification != null && !"\"notificationType\":null".equals(lastMasterNotification)) {
                 if (lastMasterNotification.equals(notificationRecord)) {
                     return;
                 }
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitorException.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitorException.java
index b12e780a0..4f7883800 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitorException.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BackUpMonitorException.java
@@ -24,6 +24,8 @@ public class BackUpMonitorException extends Exception{
     private static final long serialVersionUID = 6778134503685443473L;
     
     public BackUpMonitorException() {
+    	// Nothing for this constructor to initialize 
+    	// in an exception class.
     }
     
     public BackUpMonitorException(String message) {
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BusConsumer.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BusConsumer.java
index 2c867a3d1..6d80a6a78 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BusConsumer.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/BusConsumer.java
@@ -91,6 +91,7 @@ public interface BusConsumer {
 		/**
 		 * {@inheritDoc}
 		 */
+		@Override
 		public Iterable<String> fetch() throws MRApiException {
 			try {
                 return this.consumer.fetch();
@@ -102,6 +103,7 @@ public interface BusConsumer {
 		/**
 		 * {@inheritDoc}
 		 */
+		@Override
 		public void close() {
 			this.consumer.close();
 		}
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyAccess.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyAccess.java
index ea1514a0f..a6481aaeb 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyAccess.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyAccess.java
@@ -71,14 +71,14 @@ public class PolicyAccess implements Access {
 		if (logLevel.compareTo(level) > 0) {
             return;
         }
-		StringBuffer sb = new StringBuffer();
+		StringBuilder sb = new StringBuilder();
         sb.append(new Date()).append(' ').append(level);
         logtail(sb, args);
 	}
 
 	@Override
 	public void log(Exception e, Object... args) {
-		StringBuffer sb = new StringBuffer();
+		StringBuilder sb = new StringBuilder();
         sb.append(new Date()).append(" EXCEPTION ").append(e.getMessage());
         logtail(sb, args);
         logger.error(e.getMessage() + e);
@@ -89,7 +89,7 @@ public class PolicyAccess implements Access {
 		logLevel = level;
 	}
 	
-	private void logtail(StringBuffer sb, Object[] args) {
+	private void logtail(StringBuilder sb, Object[] args) {
         for (Object o: args) {
             String s = o.toString();
             if (s.length() > 0) {
diff --git a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyUtils.java b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyUtils.java
index 51409b6f7..fa23f4a31 100644
--- a/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyUtils.java
+++ b/PolicyEngineUtils/src/main/java/org/onap/policy/utils/PolicyUtils.java
@@ -114,7 +114,7 @@ public class PolicyUtils {
     public static String[] decodeBasicEncoding(String encodedValue) throws UnsupportedEncodingException {
         if(encodedValue!=null && encodedValue.contains("Basic ")){
             String encodedUserPassword = encodedValue.replaceFirst("Basic"  + " ", "");
-            String usernameAndPassword = null;
+            String usernameAndPassword;
             byte[] decodedBytes = Base64.getDecoder().decode(encodedUserPassword);
             usernameAndPassword = new String(decodedBytes, "UTF-8");
             StringTokenizer tokenizer = new StringTokenizer(usernameAndPassword, ":");
diff --git a/PolicyEngineUtils/src/test/java/org/onap/policy/utils/test/NotificationStoreTest.java b/PolicyEngineUtils/src/test/java/org/onap/policy/utils/test/NotificationStoreTest.java
index 7bae4239d..774bf8a7e 100644
--- a/PolicyEngineUtils/src/test/java/org/onap/policy/utils/test/NotificationStoreTest.java
+++ b/PolicyEngineUtils/src/test/java/org/onap/policy/utils/test/NotificationStoreTest.java
@@ -43,7 +43,7 @@ public class NotificationStoreTest {
     public void notificationTest() throws IOException{
         // Notification Delta test first. 
         NotificationStore.recordNotification(new StdPDPNotification());
-        assertEquals("{\"removedPolicies\":null,\"loadedPolicies\":null,\"notificationType\":null}", PolicyUtils.objectToJsonString(NotificationStore.getDeltaNotification(new StdPDPNotification())));
+        assertEquals("{\"removedPolicies\":[],\"loadedPolicies\":[],\"notificationType\":null}", PolicyUtils.objectToJsonString(NotificationStore.getDeltaNotification(new StdPDPNotification())));
         // Initialize test 
         StdPDPNotification notification = new StdPDPNotification();
         notification.setNotificationType(NotificationType.BOTH);