From: Magnusen, Drew (dm741q) Date: Thu, 21 Dec 2017 18:03:33 +0000 (-0600) Subject: Sonar cleanup for PolicyEngineUtils X-Git-Tag: v1.2.0~187^2 X-Git-Url: https://gerrit.onap.org/r/gitweb?p=policy%2Fengine.git;a=commitdiff_plain;h=7a0d068cd318eedce284c48dd46faccf2f5335ec 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) --- 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 6a558006d..23b570e9a 100644 --- 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 result = fixture.getLoadedPolicies(); // add additional test code here - assertEquals(null, result); + assertEquals(Collections.EMPTY_LIST, result); } /** @@ -204,7 +205,7 @@ public class StdPDPNotificationTest { Collection 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 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 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 PDPNotification 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 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);