From 0ae998fcb08390a0d4a2dd2b98116be280c299d5 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 23 Jul 2019 10:57:03 -0400 Subject: 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 --- .../xacml/application/common/XacmlPolicyUtils.java | 7 ++- .../common/std/StdCombinedPolicyRequest.java | 7 ++- .../common/std/StdMatchablePolicyRequest.java | 7 ++- .../xacml/application/common/std/StdOnapPip.java | 41 ++++++++------ .../std/StdXacmlApplicationServiceProvider.java | 66 +++------------------- 5 files changed, 47 insertions(+), 81 deletions(-) (limited to 'applications/common/src/main/java/org') 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 resources = decisionRequest.getResource(); for (Entry 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> 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 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 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 attributeValue = null; try { - attributeValue = DataTypes.DT_STRING.createAttributeValue(value); + attributeValue = makeString(value); } catch (Exception ex) { logger.error("Failed to convert {} to an AttributeValue", value, ex); } @@ -179,4 +174,18 @@ public abstract class StdOnapPip extends StdConfigurableEngine { } } + // these may be overridden by junit tests + + protected AttributeValue makeInteger(int value) throws DataTypeException { + return DataTypes.DT_INTEGER.createAttributeValue(value); + } + + protected AttributeValue makeLong(long value) throws DataTypeException { + return DataTypes.DT_INTEGER.createAttributeValue(value); + } + + protected AttributeValue 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 @@ -276,41 +254,6 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica return pathForData; } - /** - * 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. */ @@ -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(); + } } -- cgit 1.2.3-korg