From c19f81387f4efc666cfa45b000275f3ee7c1d9d1 Mon Sep 17 00:00:00 2001 From: Pamela Dragosh Date: Mon, 16 Sep 2019 13:09:46 -0400 Subject: 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 --- .../common/OnapPolicyFinderFactory.java | 97 ++++++++++++---------- .../common/std/StdMatchablePolicyRequest.java | 33 ++++---- .../common/std/StdMatchableTranslator.java | 54 ++++++------ .../common/std/StdMatchablePolicyRequestTest.java | 4 +- 4 files changed, 101 insertions(+), 87 deletions(-) (limited to 'applications/common/src') 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 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 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 result = gson.fromJson(stringContents.toString() ,Map.class); - // - // Find the metadata section - // - @SuppressWarnings("unchecked") - Map metadata = (Map) 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 result = gson.fromJson(stringContents.toString() ,Map.class); + // + // Find the metadata section + // + @SuppressWarnings("unchecked") + Map metadata = (Map) 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 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)); -- cgit 1.2.3-korg