Sonar xacml-pdp issues

Either log or re-throw exception
Refactor to not nest more than 3 statements
Refactor to throw at most one exception
Move variable to comply with Java Code conventions
String literal on LHS

Issue-ID: POLICY-2079
Change-Id: Iac33623ef4694cf38c4a69c8f0b9040d8439998e
Signed-off-by: Pamela Dragosh <pdragosh@research.att.com>

6 files changed, 114 insertions, 90 deletions

diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/OnapPolicyFinderFactory.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/OnapPolicyFinderFactory.java
index b955674c..e66c9943 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/OnapPolicyFinderFactory.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/OnapPolicyFinderFactory.java
@@ -219,56 +219,61 @@ public class OnapPolicyFinderFactory extends PolicyFinderFactory {
     }
 
     protected synchronized void init() {
-        if (this.needsInit) {
-            logger.info("Initializing OnapPolicyFinderFactory Properties ");
+        if (! this.needsInit) {
+            logger.info("Does not need initialization");
+            return;
+        }
+        logger.info("Initializing OnapPolicyFinderFactory Properties ");
 
-            //
-            // Check for property that combines root policies into one policyset
-            //
-            String combiningAlgorithm = properties.getProperty(
-                    ATTPDPProperties.PROP_POLICYFINDERFACTORY_COMBINEROOTPOLICIES);
-            if (combiningAlgorithm != null) {
-                try {
-                    logger.info("Combining root policies with {}", combiningAlgorithm);
-                    //
-                    // Find the combining algorithm
-                    //
-                    CombiningAlgorithm<PolicySetChild> algorithm = CombiningAlgorithmFactory.newInstance()
-                            .getPolicyCombiningAlgorithm(new IdentifierImpl(combiningAlgorithm));
-                    //
-                    // Create our root policy
-                    //
-                    PolicySet root = new PolicySet();
-                    root.setIdentifier(new IdentifierImpl(UUID.randomUUID().toString()));
-                    root.setVersion(StdVersion.newInstance("1.0"));
-                    root.setTarget(new Target());
-                    //
-                    // Set the algorithm
-                    //
-                    root.setPolicyCombiningAlgorithm(algorithm);
-                    //
-                    // Load all our root policies
-                    //
-                    for (PolicyDef policy : this.getPolicyDefs(XACMLProperties.PROP_ROOTPOLICIES)) {
-                        root.addChild(policy);
-                    }
-                    //
-                    // Set this policy as the root
-                    //
-                    this.rootPolicies = new ArrayList<>();
-                    this.rootPolicies.add(root);
-                } catch (Exception e) {
-                    logger.error("Failed to load Combining Algorithm Factory: {}", e.getLocalizedMessage());
+        //
+        // Check for property that combines root policies into one policyset
+        //
+        String combiningAlgorithm = properties.getProperty(
+                ATTPDPProperties.PROP_POLICYFINDERFACTORY_COMBINEROOTPOLICIES);
+        if (combiningAlgorithm != null) {
+            try {
+                logger.info("Combining root policies with {}", combiningAlgorithm);
+                //
+                // Find the combining algorithm
+                //
+                CombiningAlgorithm<PolicySetChild> algorithm = CombiningAlgorithmFactory.newInstance()
+                        .getPolicyCombiningAlgorithm(new IdentifierImpl(combiningAlgorithm));
+                //
+                // Create our root policy
+                //
+                PolicySet root = new PolicySet();
+                root.setIdentifier(new IdentifierImpl(UUID.randomUUID().toString()));
+                root.setVersion(StdVersion.newInstance("1.0"));
+                root.setTarget(new Target());
+                //
+                // Set the algorithm
+                //
+                root.setPolicyCombiningAlgorithm(algorithm);
+                //
+                // Load all our root policies
+                //
+                for (PolicyDef policy : this.getPolicyDefs(XACMLProperties.PROP_ROOTPOLICIES)) {
+                    root.addChild(policy);
                 }
-            } else {
-                logger.info("Loading root policies");
-                this.rootPolicies       = this.getPolicyDefs(XACMLProperties.PROP_ROOTPOLICIES);
+                //
+                // Set this policy as the root
+                //
+                this.rootPolicies = new ArrayList<>();
+                this.rootPolicies.add(root);
+            } catch (Exception e) {
+                logger.error("Failed to load Combining Algorithm Factory", e);
             }
-            this.referencedPolicies = this.getPolicyDefs(XACMLProperties.PROP_REFERENCEDPOLICIES);
-            logger.info("Root Policies: {}", this.rootPolicies.size());
-            logger.info("Referenced Policies: {}", this.referencedPolicies.size());
-            this.needsInit  = false;
+        } else {
+            logger.info("Loading root policies");
+            this.rootPolicies       = this.getPolicyDefs(XACMLProperties.PROP_ROOTPOLICIES);
         }
+        this.referencedPolicies = this.getPolicyDefs(XACMLProperties.PROP_REFERENCEDPOLICIES);
+        logger.info("Root Policies: {}", this.rootPolicies.size());
+        logger.info("Referenced Policies: {}", this.referencedPolicies.size());
+        //
+        // Make sure we set that we don't need initialization
+        //
+        this.needsInit  = false;
     }
 
     @Override
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 c32eeca4..0f3a0338 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
@@ -47,6 +47,7 @@ import lombok.Setter;
 import lombok.ToString;
 import org.onap.policy.models.decisions.concepts.DecisionRequest;
 import org.onap.policy.pdp.xacml.application.common.ToscaDictionary;
+import org.onap.policy.pdp.xacml.application.common.XacmlApplicationException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -70,21 +71,18 @@ public class StdMatchablePolicyRequest {
     @XACMLAction()
     private String action;
 
+    protected static DataTypeFactory dataTypeFactory        = null;
+
     public StdMatchablePolicyRequest() {
         super();
     }
 
-    protected static DataTypeFactory dataTypeFactory        = null;
-
     protected static synchronized DataTypeFactory getDataTypeFactory() {
         try {
             if (dataTypeFactory != null) {
                 return dataTypeFactory;
             }
             dataTypeFactory = DataTypeFactory.newInstance();
-            if (dataTypeFactory == null) {
-                LOGGER.error("Could not create data type factory");
-            }
         } catch (FactoryException e) {
             LOGGER.error("Can't get Data type Factory: {}", e);
         }
@@ -96,12 +94,10 @@ public class StdMatchablePolicyRequest {
      *
      * @param decisionRequest Input DecisionRequest
      * @return Request XACML Request object
-     * @throws DataTypeException DataType exception
-     * @throws IllegalAccessException  Illegal access exception
+     * @throws XacmlApplicationException Exception occurred parsing or creating request
      */
     @SuppressWarnings({"rawtypes", "unchecked"})
-    public static Request createInstance(DecisionRequest decisionRequest) throws IllegalAccessException,
-        DataTypeException {
+    public static Request createInstance(DecisionRequest decisionRequest) throws XacmlApplicationException {
         //
         // Create our request object
         //
@@ -120,7 +116,12 @@ public class StdMatchablePolicyRequest {
         // Parse the request - we use the annotations to create a
         // basic XACML request.
         //
-        Request xacmlRequest = RequestParser.parseRequest(request);
+        Request xacmlRequest;
+        try {
+            xacmlRequest = RequestParser.parseRequest(request);
+        } catch (IllegalAccessException | DataTypeException e) {
+            throw new XacmlApplicationException("Could not parse request " + e.getLocalizedMessage());
+        }
         //
         // Create an object we can add to
         //
@@ -137,10 +138,14 @@ public class StdMatchablePolicyRequest {
             // Its possible we may have to load the policy type model
             // and use that to validate the fields that are matchable.
             //
-            if (entrySet.getValue() instanceof Collection) {
-                addResources(resourceAttributes, (Collection) entrySet.getValue(), entrySet.getKey());
-            } else {
-                addResources(resourceAttributes, Arrays.asList(entrySet.getValue().toString()), entrySet.getKey());
+            try {
+                if (entrySet.getValue() instanceof Collection) {
+                    addResources(resourceAttributes, (Collection) entrySet.getValue(), entrySet.getKey());
+                } else {
+                    addResources(resourceAttributes, Arrays.asList(entrySet.getValue().toString()), entrySet.getKey());
+                }
+            } catch (DataTypeException e) {
+                throw new XacmlApplicationException("Failed to add resource " + e.getLocalizedMessage());
             }
         }
         mutableRequest.add(resourceAttributes);
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java
index 7142d43d..7551f7e3 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java
@@ -23,7 +23,6 @@
 package org.onap.policy.pdp.xacml.application.common.std;
 
 import com.att.research.xacml.api.AttributeAssignment;
-import com.att.research.xacml.api.DataTypeException;
 import com.att.research.xacml.api.Decision;
 import com.att.research.xacml.api.Identifier;
 import com.att.research.xacml.api.Obligation;
@@ -77,6 +76,7 @@ import org.onap.policy.pdp.xacml.application.common.ToscaDictionary;
 import org.onap.policy.pdp.xacml.application.common.ToscaPolicyConversionException;
 import org.onap.policy.pdp.xacml.application.common.ToscaPolicyTranslator;
 import org.onap.policy.pdp.xacml.application.common.ToscaPolicyTranslatorUtils;
+import org.onap.policy.pdp.xacml.application.common.XacmlApplicationException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -108,7 +108,7 @@ public class StdMatchableTranslator implements ToscaPolicyTranslator {
         LOGGER.info("Converting Request {}", request);
         try {
             return StdMatchablePolicyRequest.createInstance(request);
-        } catch (IllegalArgumentException | IllegalAccessException | DataTypeException e) {
+        } catch (XacmlApplicationException e) {
             LOGGER.error("Failed to convert DecisionRequest: {}", e);
         }
         //
@@ -158,31 +158,35 @@ public class StdMatchableTranslator implements ToscaPolicyTranslator {
                 //
                 // We care about the content attribute
                 //
-                if (ToscaDictionary.ID_OBLIGATION_POLICY_MONITORING_CONTENTS
+                if (! ToscaDictionary.ID_OBLIGATION_POLICY_MONITORING_CONTENTS
                         .equals(assignment.getAttributeId())) {
                     //
-                    // The contents are in Json form
+                    // If its not there, move on
                     //
-                    Object stringContents = assignment.getAttributeValue().getValue();
-                    if (LOGGER.isInfoEnabled()) {
-                        LOGGER.info("Policy contents: {}{}", System.lineSeparator(), stringContents);
-                    }
-                    //
-                    // Let's parse it into a map using Gson
-                    //
-                    Gson gson = new Gson();
-                    @SuppressWarnings("unchecked")
-                    Map<String, Object> result = gson.fromJson(stringContents.toString() ,Map.class);
-                    //
-                    // Find the metadata section
-                    //
-                    @SuppressWarnings("unchecked")
-                    Map<String, Object> metadata = (Map<String, Object>) result.get("metadata");
-                    if (metadata != null) {
-                        decisionResponse.getPolicies().put(metadata.get(POLICY_ID).toString(), result);
-                    } else {
-                        LOGGER.error("Missing metadata section in policy contained in obligation.");
-                    }
+                    continue;
+                }
+                //
+                // The contents are in Json form
+                //
+                Object stringContents = assignment.getAttributeValue().getValue();
+                if (LOGGER.isInfoEnabled()) {
+                    LOGGER.info("Policy contents: {}{}", System.lineSeparator(), stringContents);
+                }
+                //
+                // Let's parse it into a map using Gson
+                //
+                Gson gson = new Gson();
+                @SuppressWarnings("unchecked")
+                Map<String, Object> result = gson.fromJson(stringContents.toString() ,Map.class);
+                //
+                // Find the metadata section
+                //
+                @SuppressWarnings("unchecked")
+                Map<String, Object> metadata = (Map<String, Object>) result.get("metadata");
+                if (metadata != null) {
+                    decisionResponse.getPolicies().put(metadata.get(POLICY_ID).toString(), result);
+                } else {
+                    LOGGER.error("Missing metadata section in policy contained in obligation.");
                 }
             }
         }
@@ -340,7 +344,7 @@ public class StdMatchableTranslator implements ToscaPolicyTranslator {
                     continue;
                 }
                 for (Entry<String, String> entrySet : propertiesEntry.getValue().getMetadata().entrySet()) {
-                    if (entrySet.getKey().equals("matchable") && entrySet.getValue().equals("true")) {
+                    if ("matchable".equals(entrySet.getKey()) && "true".equals(entrySet.getValue())) {
                         return true;
                     }
                 }
diff --git a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequestTest.java b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequestTest.java
index 1c844621..d3e362c1 100644
--- a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequestTest.java
+++ b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchablePolicyRequestTest.java
@@ -24,7 +24,6 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.when;
 
-import com.att.research.xacml.api.DataTypeException;
 import com.att.research.xacml.api.Request;
 import com.att.research.xacml.api.RequestAttributes;
 import com.att.research.xacml.api.XACML3;
@@ -39,6 +38,7 @@ import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.onap.policy.models.decisions.concepts.DecisionRequest;
 import org.onap.policy.pdp.xacml.application.common.ToscaDictionary;
+import org.onap.policy.pdp.xacml.application.common.XacmlApplicationException;
 
 public class StdMatchablePolicyRequestTest {
     private static final String ACTION = "my-action";
@@ -74,7 +74,7 @@ public class StdMatchablePolicyRequestTest {
     }
 
     @Test
-    public void testCreateInstance() throws IllegalAccessException, DataTypeException {
+    public void testCreateInstance() throws XacmlApplicationException {
         resources.put("resource1", RESOURCE1);
         resources.put("resource2", RESOURCE2);
         resources.put("resource3", Arrays.asList(RESOURCE3, RESOURCE4));
diff --git a/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpRestController.java b/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpRestController.java
index 3de830e3..5b17c34e 100644
--- a/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpRestController.java
+++ b/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpRestController.java
@@ -55,6 +55,8 @@ import org.onap.policy.pdpx.main.rest.model.StatisticsReport;
 import org.onap.policy.pdpx.main.rest.provider.DecisionProvider;
 import org.onap.policy.pdpx.main.rest.provider.HealthCheckProvider;
 import org.onap.policy.pdpx.main.rest.provider.StatisticsProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Class to provide xacml pdp REST services.
@@ -70,8 +72,10 @@ import org.onap.policy.pdpx.main.rest.provider.StatisticsProvider;
         schemes = {SwaggerDefinition.Scheme.HTTP, SwaggerDefinition.Scheme.HTTPS},
         securityDefinition = @SecurityDefinition(basicAuthDefinitions = {@BasicAuthDefinition(key = "basicAuth")}))
 public class XacmlPdpRestController {
+    private static final Logger LOGGER = LoggerFactory.getLogger(XacmlPdpRestController.class);
     public static final String APPLICATION_YAML = "application/yaml";
 
+
     @GET
     @Path("/healthcheck")
     @ApiOperation(value = "Perform a system healthcheck",
@@ -176,6 +180,7 @@ public class XacmlPdpRestController {
             return addLoggingHeaders(addVersionControlHeaders(Response.status(Response.Status.OK)), requestId)
                     .entity(new DecisionProvider().fetchDecision(body)).build();
         } catch (DecisionException e) {
+            LOGGER.error("Decision exception", e);
             XacmlPdpStatisticsManager.getCurrent().updateErrorCount();
             return addLoggingHeaders(
                     addVersionControlHeaders(Response.status((e.getErrorResponse().getResponseCode()))), requestId)
diff --git a/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java b/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java
index fa7459dc..16bea1d1 100644
--- a/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java
+++ b/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java
@@ -50,11 +50,10 @@ public class TestUtils {
      *
      * @param resourceFile resource file
      * @param service XacmlApplicationServiceProvider
-     * @throws CoderException exception if it cannot be decoded
      * @throws XacmlApplicationException If the application cannot load the policy
      */
     public static List<ToscaPolicy> loadPolicies(String resourceFile, XacmlApplicationServiceProvider service)
-            throws CoderException, XacmlApplicationException {
+            throws XacmlApplicationException {
         //
         // Our return object
         //
@@ -66,7 +65,13 @@ public class TestUtils {
         //
         // Serialize it into a class
         //
-        ToscaServiceTemplate serviceTemplate = yamlCoder.decode(policyYaml, ToscaServiceTemplate.class);
+        ToscaServiceTemplate serviceTemplate;
+        try {
+            serviceTemplate = yamlCoder.decode(policyYaml, ToscaServiceTemplate.class);
+        } catch (CoderException e) {
+            throw new XacmlApplicationException("Failed to decode policy from resource file "
+                + e.getLocalizedMessage());
+        }
         //
         // Make sure all the fields are setup properly
         //