Fix sonar issues in drools-applications database

Refactored to eliminate duplicate code blocks.
Added coverage tests.

Change-Id: I99d2b6f68fee38a1dbbf038ad29d1dfe87fb4ae7
Issue-ID: POLICY-1791
Signed-off-by: Jim Hahn <jrh3@att.com>

3 files changed, 106 insertions, 108 deletions

diff --git a/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/CountRecentOperationsPip.java b/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/CountRecentOperationsPip.java
index 1f73ed3ce..7b6f13611 100644
--- a/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/CountRecentOperationsPip.java
+++ b/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/CountRecentOperationsPip.java
@@ -27,11 +27,12 @@ import com.att.research.xacml.api.pip.PIPRequest;
 import com.att.research.xacml.api.pip.PIPResponse;
 import com.att.research.xacml.std.pip.StdMutablePIPResponse;
 import com.att.research.xacml.std.pip.StdPIPResponse;
-import com.google.common.base.Strings;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.Properties;
-import javax.persistence.Persistence;
+import java.util.Set;
 import org.onap.policy.database.ToscaDictionary;
 import org.onap.policy.database.std.StdOnapPip;
 import org.slf4j.Logger;
@@ -42,6 +43,9 @@ public class CountRecentOperationsPip extends StdOnapPip {
     public static final String ISSUER_NAME = "count-recent-operations";
     private static Logger logger = LoggerFactory.getLogger(CountRecentOperationsPip.class);
 
+    private static final Set<String> TIME_WINDOW_SCALES = Collections
+                    .unmodifiableSet(new HashSet<>(Arrays.asList("minute", "hour", "day", "week", "month", "year")));
+
     public CountRecentOperationsPip() {
         super();
     }
@@ -53,25 +57,7 @@ public class CountRecentOperationsPip extends StdOnapPip {
 
     @Override
     public void configure(String id, Properties properties) throws PIPException {
-        super.configure(id, properties);
-        //
-        // Create our entity manager
-        //
-        em = null;
-        try {
-            //
-            // In case there are any overloaded properties for the JPA
-            //
-            Properties emProperties = new Properties(properties);
-            //
-            // Create the entity manager factory
-            //
-            em = Persistence.createEntityManagerFactory(
-                    properties.getProperty(ISSUER_NAME + ".persistenceunit"),
-                    emProperties).createEntityManager();
-        } catch (Exception e) {
-            logger.error("Persistence failed {} operations history db {}", e.getLocalizedMessage(), e);
-        }
+        super.configure(id, properties, ISSUER_NAME);
     }
 
     /**
@@ -85,23 +71,11 @@ public class CountRecentOperationsPip extends StdOnapPip {
     public PIPResponse getAttributes(PIPRequest pipRequest, PIPFinder pipFinder) throws PIPException {
         logger.debug("getAttributes requesting attribute {} of type {} for issuer {}",
                 pipRequest.getAttributeId(), pipRequest.getDataTypeId(), pipRequest.getIssuer());
-        //
-        // Determine if the issuer is correct
-        //
-        if (Strings.isNullOrEmpty(pipRequest.getIssuer())) {
-            logger.debug("issuer is null - returning empty response");
-            //
-            // We only respond to ourself as the issuer
-            //
-            return StdPIPResponse.PIP_RESPONSE_EMPTY;
-        }
-        if (! pipRequest.getIssuer().startsWith(ToscaDictionary.GUARD_ISSUER_PREFIX)) {
-            logger.debug("Issuer does not start with guard");
-            //
-            // We only respond to ourself as the issuer
-            //
+
+        if (isRequestInvalid(pipRequest)) {
             return StdPIPResponse.PIP_RESPONSE_EMPTY;
         }
+
         //
         // Parse out the issuer which denotes the time window
         // Eg: any-prefix:tw:10:minute
@@ -156,12 +130,7 @@ public class CountRecentOperationsPip extends StdOnapPip {
         //
         // Compute the time window
         //
-        if (! "minute".equalsIgnoreCase(timeWindowScale)
-            && ! "hour".equalsIgnoreCase(timeWindowScale)
-            && ! "day".equalsIgnoreCase(timeWindowScale)
-            && ! "week".equalsIgnoreCase(timeWindowScale)
-            && ! "month".equalsIgnoreCase(timeWindowScale)
-            && ! "year".equalsIgnoreCase(timeWindowScale)) {
+        if (! TIME_WINDOW_SCALES.contains(timeWindowScale.toLowerCase())) {
             //
             // Unsupported
             //
@@ -197,7 +166,7 @@ public class CountRecentOperationsPip extends StdOnapPip {
                 .setParameter(4, timeWindowScale)
                 .setParameter(5, timeWindowVal * -1)
                 .getSingleResult();
-        } catch (Exception e) {
+        } catch (RuntimeException e) {
             logger.error("Named query failed ", e);
         }
         //
@@ -209,10 +178,10 @@ public class CountRecentOperationsPip extends StdOnapPip {
             //
             logger.info("operations query returned {}", result);
             //
-            // Should get back a long
+            // Should get back a number
             //
-            if (result instanceof Long) {
-                return ((Long) result).intValue();
+            if (result instanceof Number) {
+                return ((Number) result).intValue();
             }
             //
             // We shouldn't really get this result, but just
diff --git a/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/GetOperationOutcomePip.java b/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/GetOperationOutcomePip.java
index 20c8f028a..5a0db0501 100644
--- a/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/GetOperationOutcomePip.java
+++ b/controlloop/common/database/src/main/java/org/onap/policy/database/operationshistory/GetOperationOutcomePip.java
@@ -25,12 +25,10 @@ import com.att.research.xacml.api.pip.PIPRequest;
 import com.att.research.xacml.api.pip.PIPResponse;
 import com.att.research.xacml.std.pip.StdMutablePIPResponse;
 import com.att.research.xacml.std.pip.StdPIPResponse;
-import com.google.common.base.Strings;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Properties;
 import javax.persistence.NoResultException;
-import javax.persistence.Persistence;
 import org.onap.policy.database.ToscaDictionary;
 import org.onap.policy.database.std.StdOnapPip;
 import org.slf4j.Logger;
@@ -52,25 +50,7 @@ public class GetOperationOutcomePip extends StdOnapPip {
 
     @Override
     public void configure(String id, Properties properties) throws PIPException {
-        super.configure(id, properties);
-        //
-        // Create our entity manager
-        //
-        em = null;
-        try {
-            //
-            // In case there are any overloaded properties for the JPA
-            //
-            Properties emProperties = new Properties(properties);
-            //
-            // Create the entity manager factory
-            //
-            em = Persistence.createEntityManagerFactory(
-                    properties.getProperty(ISSUER_NAME + ".persistenceunit"),
-                    emProperties).createEntityManager();
-        } catch (Exception e) {
-            logger.error("Persistence failed {} operations history db {}", e.getLocalizedMessage(), e);
-        }
+        super.configure(id, properties, ISSUER_NAME);
     }
 
     /**
@@ -84,31 +64,28 @@ public class GetOperationOutcomePip extends StdOnapPip {
     public PIPResponse getAttributes(PIPRequest pipRequest, PIPFinder pipFinder) throws PIPException {
         logger.debug("getAttributes requesting attribute {} of type {} for issuer {}",
                 pipRequest.getAttributeId(), pipRequest.getDataTypeId(), pipRequest.getIssuer());
-        //
-        // Determine if the issuer is correct
-        //
-        if (Strings.isNullOrEmpty(pipRequest.getIssuer())) {
-            logger.debug("issuer is null - returning empty response");
-            //
-            // We only respond to ourself as the issuer
-            //
-            return StdPIPResponse.PIP_RESPONSE_EMPTY;
-        }
-        if (! pipRequest.getIssuer().startsWith(ToscaDictionary.GUARD_ISSUER_PREFIX)) {
-            logger.debug("Issuer does not start with guard");
-            //
-            // We only respond to ourself as the issuer
-            //
+
+        if (isRequestInvalid(pipRequest)) {
             return StdPIPResponse.PIP_RESPONSE_EMPTY;
         }
+
         //
         // Parse out the issuer which denotes the time window
         // Eg: any-prefix:clname:some-controlloop-name
         //
         String[] s1 = pipRequest.getIssuer().split("clname:");
         String clname = s1[1];
-        String target = null;
-        target = getTarget(pipFinder);
+        String target = getTarget(pipFinder);
+        //
+        // Sanity check
+        //
+        if (target == null) {
+            //
+            // See if we have all the values
+            //
+            logger.error("missing attributes return empty");
+            return StdPIPResponse.PIP_RESPONSE_EMPTY;
+        }
 
         logger.debug("Going to query DB about: clname={}, target={}", clname, target);
         String outcome = doDatabaseQuery(clname, target);
diff --git a/controlloop/common/database/src/main/java/org/onap/policy/database/std/StdOnapPip.java b/controlloop/common/database/src/main/java/org/onap/policy/database/std/StdOnapPip.java
index a94727371..1416b3ef9 100644
--- a/controlloop/common/database/src/main/java/org/onap/policy/database/std/StdOnapPip.java
+++ b/controlloop/common/database/src/main/java/org/onap/policy/database/std/StdOnapPip.java
@@ -39,6 +39,8 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.Properties;
 import javax.persistence.EntityManager;
+import javax.persistence.Persistence;
+import org.apache.commons.lang3.StringUtils;
 import org.onap.policy.database.ToscaDictionary;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -74,11 +76,65 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
         return Collections.emptyList();
     }
 
-    @Override
-    public void configure(String id, Properties properties) throws PIPException {
+    /**
+     * Configures this object and initializes {@link #em}.
+     *
+     * @param id name of this engine
+     * @param properties configuration properties
+     * @param issuerName name of this issuer, used to identify the persistence unit
+     * @throws PIPException if an error occurs
+     */
+    protected void configure(String id, Properties properties, String issuerName) throws PIPException {
         super.configure(id, properties);
         logger.debug("Configuring historyDb PIP {}", properties);
         this.properties = properties;
+
+        //
+        // Create our entity manager
+        //
+        em = null;
+        try {
+            //
+            // In case there are any overloaded properties for the JPA
+            //
+            Properties emProperties = new Properties(properties);
+            //
+            // Create the entity manager factory
+            //
+            em = Persistence.createEntityManagerFactory(
+                    properties.getProperty(issuerName + ".persistenceunit"),
+                    emProperties).createEntityManager();
+        } catch (Exception e) {
+            logger.error("Persistence failed {} operations history db {}", e.getLocalizedMessage(), e);
+        }
+    }
+
+    /**
+     * Determines if a request is valid.
+     *
+     * @param pipRequest request to validate
+     * @return {@code true} if the request is <i>NOT</i> valid, {@code false} if it is
+     */
+    protected boolean isRequestInvalid(PIPRequest pipRequest) {
+        //
+        // Determine if the issuer is correct
+        //
+        if (StringUtils.isBlank(pipRequest.getIssuer())) {
+            logger.debug("issuer is null - returning empty response");
+            //
+            // We only respond to ourself as the issuer
+            //
+            return true;
+        }
+        if (! pipRequest.getIssuer().startsWith(ToscaDictionary.GUARD_ISSUER_PREFIX)) {
+            logger.debug("Issuer does not start with guard");
+            //
+            // We only respond to ourself as the issuer
+            //
+            return true;
+        }
+
+        return false;
     }
 
     protected String getActor(PIPFinder pipFinder) {
@@ -150,14 +206,16 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
     }
 
     protected String findFirstAttributeValue(PIPResponse pipResponse) {
-        for (Attribute attribute: pipResponse.getAttributes()) {
-            Iterator<AttributeValue<String>> iterAttributeValues    = attribute.findValues(DataTypes.DT_STRING);
-            if (iterAttributeValues != null) {
-                while (iterAttributeValues.hasNext()) {
-                    String value   = iterAttributeValues.next().getValue();
-                    if (value != null) {
-                        return value;
-                    }
+        for (Attribute attribute : pipResponse.getAttributes()) {
+            Iterator<AttributeValue<String>> iterAttributeValues = attribute.findValues(DataTypes.DT_STRING);
+            if (iterAttributeValues == null) {
+                continue;
+            }
+
+            while (iterAttributeValues.hasNext()) {
+                String value = iterAttributeValues.next().getValue();
+                if (value != null) {
+                    return value;
                 }
             }
         }
@@ -165,31 +223,25 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
     }
 
     protected void addIntegerAttribute(StdMutablePIPResponse stdPipResponse, Identifier category,
-            Identifier attributeId, int value, PIPRequest pipRequest) {
-        AttributeValue<BigInteger> attributeValue   = null;
+                    Identifier attributeId, int value, PIPRequest pipRequest) {
         try {
-            attributeValue  = DataTypes.DT_INTEGER.createAttributeValue(value);
+            AttributeValue<BigInteger> attributeValue = DataTypes.DT_INTEGER.createAttributeValue(value);
+            stdPipResponse.addAttribute(new StdMutableAttribute(category, attributeId, attributeValue,
+                            pipRequest.getIssuer(), false));
         } catch (Exception e) {
             logger.error("Failed to convert {} to integer {}", value, e);
         }
-        if (attributeValue != null) {
-            stdPipResponse.addAttribute(new StdMutableAttribute(category, attributeId, attributeValue,
-                    pipRequest.getIssuer(), false));
-        }
     }
 
     protected void addStringAttribute(StdMutablePIPResponse stdPipResponse, Identifier category, Identifier attributeId,
-            String value, PIPRequest pipRequest) {
-        AttributeValue<String> attributeValue = null;
+                    String value, PIPRequest pipRequest) {
         try {
-            attributeValue = DataTypes.DT_STRING.createAttributeValue(value);
+            AttributeValue<String> attributeValue = DataTypes.DT_STRING.createAttributeValue(value);
+            stdPipResponse.addAttribute(new StdMutableAttribute(category, attributeId, attributeValue,
+                            pipRequest.getIssuer(), false));
         } catch (Exception ex) {
             logger.error("Failed to convert {} to an AttributeValue<String>", value, ex);
         }
-        if (attributeValue != null) {
-            stdPipResponse.addAttribute(new StdMutableAttribute(category, attributeId, attributeValue,
-                    pipRequest.getIssuer(), false));
-        }
     }
 
 }