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>

diff --git a/PolicyEngineAPI/src/test/java/org/onap/policy/std/test/StdPDPNotificationTest.java b/PolicyEngineAPI/src/test/java/org/onap/policy/std/test/StdPDPNotificationTest.java
index 6a55800..23b570e 100644 (file)
--- a/PolicyEngineAPI/src/test/java/org/onap/policy/std/test/StdPDPNotificationTest.java
+++ b/PolicyEngineAPI/src/test/java/org/onap/policy/std/test/StdPDPNotificationTest.java
@@ -21,6 +21,7 @@
 package org.onap.policy.std.test;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.LinkedList;
 
 import org.junit.*;
@@ -115,7 +116,7 @@ public class StdPDPNotificationTest {
                Collection<LoadedPolicy> result = fixture.getLoadedPolicies();
 
                // add additional test code here
-               assertEquals(null, result);
+               assertEquals(Collections.EMPTY_LIST, result);
        }
 
        /**
@@ -204,7 +205,7 @@ public class StdPDPNotificationTest {
                Collection<RemovedPolicy> result = fixture.getRemovedPolicies();
 
                // add additional test code here
-               assertEquals(null, result);
+               assertEquals(Collections.EMPTY_LIST, result);
        }
 
        /**

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 8c6c958..47b2360 100644 (file)
--- 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 10268f6..ebbed21 100644 (file)
--- 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 1640fe8..54b9bb0 100644 (file)
--- 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 5430e24..04ec698 100644 (file)
--- 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 ecb51b2..9d5222a 100644 (file)
--- 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 d476d2c..b67b022 100644 (file)
--- 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 f45ba56..4dfc64c 100644 (file)
--- 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 9aab2ad..1353cc6 100644 (file)
--- 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 b12e780..4f78838 100644 (file)
--- 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 2c867a3..6d80a6a 100644 (file)
--- 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 ea1514a..a6481aa 100644 (file)
--- 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 51409b6..fa23f4a 100644 (file)
--- 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 7bae423..774bf8a 100644 (file)
--- 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);