Add more junit coverage to xacml-pdp (round #2)

Also removed unused methods.
Also extracted constants.

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

5 files changed, 47 insertions, 81 deletions

diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlPolicyUtils.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlPolicyUtils.java
index c12aae2c..6f2a9f0e 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlPolicyUtils.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlPolicyUtils.java
@@ -52,6 +52,9 @@ import org.slf4j.LoggerFactory;
 public class XacmlPolicyUtils {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(XacmlPolicyUtils.class);
+
+    public static final String XACML_PROPERTY_FILE = "xacml.properties";
+
     private static final String DOT_FILE_SUFFIX = ".file";
     private static final String NOT_FOUND_MESSAGE = "NOT FOUND";
 
@@ -403,7 +406,7 @@ public class XacmlPolicyUtils {
      * @return Path to rootPath/xacml.properties file
      */
     public static Path getPropertiesPath(Path rootPath) {
-        return Paths.get(rootPath.toAbsolutePath().toString(), "xacml.properties");
+        return Paths.get(rootPath.toAbsolutePath().toString(), XACML_PROPERTY_FILE);
     }
 
     @FunctionalInterface
@@ -434,7 +437,7 @@ public class XacmlPolicyUtils {
             //
             // Now we create a new xacml.properties in the temporary folder location
             //
-            File propertiesFile = creator.createAFile("xacml.properties");
+            File propertiesFile = creator.createAFile(XACML_PROPERTY_FILE);
             //
             // Iterate through any root policies defined
             //
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyRequest.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyRequest.java
index 1acf741a..d7cf3f2d 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyRequest.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyRequest.java
@@ -43,6 +43,9 @@ import org.onap.policy.models.decisions.concepts.DecisionRequest;
 @XACMLRequest(ReturnPolicyIdList = true)
 public class StdCombinedPolicyRequest {
 
+    public static final String POLICY_TYPE_KEY = "policy-type";
+    public static final String POLICY_ID_KEY = "policy-id";
+
     @XACMLSubject(includeInResults = true)
     private String onapName;
 
@@ -92,7 +95,7 @@ public class StdCombinedPolicyRequest {
         //
         Map<String, Object> resources = decisionRequest.getResource();
         for (Entry<String, Object> entrySet : resources.entrySet()) {
-            if ("policy-id".equals(entrySet.getKey())) {
+            if (POLICY_ID_KEY.equals(entrySet.getKey())) {
                 if (entrySet.getValue() instanceof Collection) {
                     addPolicyIds(request, (Collection) entrySet.getValue());
                 } else if (entrySet.getValue() instanceof String) {
@@ -100,7 +103,7 @@ public class StdCombinedPolicyRequest {
                 }
                 continue;
             }
-            if ("policy-type".equals(entrySet.getKey())) {
+            if (POLICY_TYPE_KEY.equals(entrySet.getKey())) {
                 if (entrySet.getValue() instanceof Collection) {
                     addPolicyTypes(request, (Collection) entrySet.getValue());
                 } else if (entrySet.getValue() instanceof String) {
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequest.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequest.java
index 086d21db..0e5edf84 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequest.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequest.java
@@ -44,6 +44,9 @@ import org.onap.policy.models.decisions.concepts.DecisionRequest;
 @XACMLRequest(ReturnPolicyIdList = true)
 public class StdMatchablePolicyRequest {
 
+    public static final String POLICY_TYPE_KEY = "policyType";
+    public static final String POLICY_SCOPE_KEY = "policyScope";
+
     @XACMLSubject(includeInResults = true)
     private String onapName;
 
@@ -103,7 +106,7 @@ public class StdMatchablePolicyRequest {
             // Its possible we may have to load the policy type model
             // and use that to find the fields that are matchable.
             //
-            if ("policyScope".equals(entrySet.getKey())) {
+            if (POLICY_SCOPE_KEY.equals(entrySet.getKey())) {
                 if (entrySet.getValue() instanceof Collection) {
                     addPolicyScopes(request, (Collection) entrySet.getValue());
                 } else if (entrySet.getValue() instanceof String) {
@@ -111,7 +114,7 @@ public class StdMatchablePolicyRequest {
                 }
                 continue;
             }
-            if ("policyType".equals(entrySet.getKey())) {
+            if (POLICY_TYPE_KEY.equals(entrySet.getKey())) {
                 if (entrySet.getValue() instanceof Collection) {
                     addPolicyTypes(request, (Collection) entrySet.getValue());
                 }
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java
index 5336d572..c511a9ad 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java
@@ -22,6 +22,7 @@ package org.onap.policy.pdp.xacml.application.common.std;
 
 import com.att.research.xacml.api.Attribute;
 import com.att.research.xacml.api.AttributeValue;
+import com.att.research.xacml.api.DataTypeException;
 import com.att.research.xacml.api.Identifier;
 import com.att.research.xacml.api.XACML3;
 import com.att.research.xacml.api.pip.PIPException;
@@ -104,16 +105,12 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
         try {
             pipResponse = pipFinder.getMatchingAttributes(pipRequest, this);
             if (pipResponse.getStatus() != null && !pipResponse.getStatus().isOk()) {
-                if (logger.isInfoEnabled()) {
-                    logger.info("get attribute error retrieving {}: {}", pipRequest.getAttributeId().stringValue(),
-                        pipResponse.getStatus());
-                }
+                logger.info("get attribute error retrieving {}: {}", pipRequest.getAttributeId().stringValue(),
+                                pipResponse.getStatus());
                 pipResponse = null;
             }
             if (pipResponse != null && pipResponse.getAttributes().isEmpty()) {
-                if (logger.isInfoEnabled()) {
-                    logger.info("No value for {}", pipRequest.getAttributeId().stringValue());
-                }
+                logger.info("No value for {}", pipRequest.getAttributeId().stringValue());
                 pipResponse = null;
             }
         } catch (PIPException ex) {
@@ -125,12 +122,10 @@ 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;
-                    }
+            while (iterAttributeValues.hasNext()) {
+                String value   = iterAttributeValues.next().getValue();
+                if (value != null) {
+                    return value;
                 }
             }
         }
@@ -141,7 +136,7 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
             Identifier attributeId, int value, PIPRequest pipRequest) {
         AttributeValue<BigInteger> attributeValue   = null;
         try {
-            attributeValue  = DataTypes.DT_INTEGER.createAttributeValue(value);
+            attributeValue  = makeInteger(value);
         } catch (Exception e) {
             logger.error("Failed to convert {} to integer {}", value, e);
         }
@@ -155,7 +150,7 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
             Identifier attributeId, long value, PIPRequest pipRequest) {
         AttributeValue<BigInteger> attributeValue   = null;
         try {
-            attributeValue  = DataTypes.DT_INTEGER.createAttributeValue(value);
+            attributeValue  = makeLong(value);
         } catch (Exception e) {
             logger.error("Failed to convert {} to long {}", value, e);
         }
@@ -169,7 +164,7 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
             String value, PIPRequest pipRequest) {
         AttributeValue<String> attributeValue = null;
         try {
-            attributeValue = DataTypes.DT_STRING.createAttributeValue(value);
+            attributeValue = makeString(value);
         } catch (Exception ex) {
             logger.error("Failed to convert {} to an AttributeValue<String>", value, ex);
         }
@@ -179,4 +174,18 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
         }
     }
 
+    // these may be overridden by junit tests
+
+    protected AttributeValue<BigInteger> makeInteger(int value) throws DataTypeException {
+        return DataTypes.DT_INTEGER.createAttributeValue(value);
+    }
+
+    protected AttributeValue<BigInteger> makeLong(long value) throws DataTypeException {
+        return DataTypes.DT_INTEGER.createAttributeValue(value);
+    }
+
+    protected AttributeValue<String> makeString(String value) throws DataTypeException {
+        return DataTypes.DT_STRING.createAttributeValue(value);
+    }
+
 }
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java
index 1dd30ec4..89299567 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java
@@ -29,21 +29,15 @@ import com.att.research.xacml.api.pdp.PDPEngineFactory;
 import com.att.research.xacml.api.pdp.PDPException;
 import com.att.research.xacml.util.FactoryException;
 import com.att.research.xacml.util.XACMLPolicyWriter;
-
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.PolicyType;
-
 import org.apache.commons.lang3.tuple.Pair;
 import org.onap.policy.models.decisions.concepts.DecisionRequest;
 import org.onap.policy.models.decisions.concepts.DecisionResponse;
@@ -87,29 +81,13 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica
         this.pathForData = pathForData;
         LOGGER.info("New Path is {}", this.pathForData.toAbsolutePath());
         //
-        // Ensure properties exist
-        //
-        Path propertiesPath = XacmlPolicyUtils.getPropertiesPath(pathForData);
-        if (! propertiesPath.toFile().exists()) {
-            LOGGER.info("Copying src/main/resources/xacml.properties to path");
-            //
-            // Properties do not exist, by default we will copy ours over
-            // from src/main/resources
-            //
-            try {
-                Files.copy(Paths.get("src/main/resources/xacml.properties"), propertiesPath);
-            } catch (IOException e) {
-                throw new XacmlApplicationException("Failed to copy xacml.propertis", e);
-            }
-        }
-        //
         // Look for and load the properties object
         //
         try {
             pdpProperties = XacmlPolicyUtils.loadXacmlProperties(XacmlPolicyUtils.getPropertiesPath(pathForData));
             LOGGER.info("{}", pdpProperties);
         } catch (IOException e) {
-            throw new XacmlApplicationException("Failed to load xacml.propertis", e);
+            throw new XacmlApplicationException("Failed to load " + XacmlPolicyUtils.XACML_PROPERTY_FILE, e);
         }
         //
         // Create an engine
@@ -277,41 +255,6 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica
     }
 
     /**
-     * Load properties from given file.
-     *
-     * @throws IOException If unable to read file
-     */
-    protected synchronized Properties loadXacmlProperties() throws IOException {
-        LOGGER.info("Loading xacml properties {}", pathForData);
-        try (InputStream is = Files.newInputStream(pathForData)) {
-            Properties properties = new Properties();
-            properties.load(is);
-            return properties;
-        }
-    }
-
-    /**
-     * Stores the XACML Properties to the given file location.
-     *
-     * @throws IOException If unable to store the file.
-     */
-    protected synchronized void storeXacmlProperties() throws IOException {
-        try (OutputStream os = Files.newOutputStream(pathForData)) {
-            String strComments = "#";
-            pdpProperties.store(os, strComments);
-        }
-    }
-
-    /**
-     * Appends 'xacml.properties' to a root Path object
-     *
-     * @return Path to rootPath/xacml.properties file
-     */
-    protected synchronized Path getPropertiesPath() {
-        return Paths.get(pathForData.toAbsolutePath().toString(), "xacml.properties");
-    }
-
-    /**
      * Creates an instance of PDP engine given the Properties object.
      */
     protected synchronized void createEngine(Properties properties) {
@@ -319,7 +262,7 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica
         // Now initialize the XACML PDP Engine
         //
         try {
-            PDPEngineFactory factory = PDPEngineFactory.newInstance();
+            PDPEngineFactory factory = getPdpEngineFactory();
             PDPEngine engine = factory.newEngine(properties);
             if (engine != null) {
                 this.pdpEngine = engine;
@@ -358,4 +301,9 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica
         return response;
     }
 
+    // these may be overridden by junit tests
+
+    protected PDPEngineFactory getPdpEngineFactory() throws FactoryException {
+        return PDPEngineFactory.newInstance();
+    }
 }