From cad452cdb6d999f822fe11dc4fdd232cce30ff6a Mon Sep 17 00:00:00 2001 From: liamfallon Date: Tue, 6 Nov 2018 12:02:46 +0000 Subject: Refactor unit test data There were many copies of test policies and examples strewn through the Apex unit tests. This change cleans up the unit tests so that a single version of all example policies is used in all tests. Also added a new relative file root command line parameter to Apex to allow the root of relative paths in configuration files to be set. Apologies for the size of this review but unfortunately all of this must be done in one shot. Issue-ID: POLICY-1252 Change-Id: Ibbb18fbf18e3897a1c61301d0a65e62bc643a0e9 Signed-off-by: liamfallon (cherry picked from commit 53d8916cc60d97e2ce7ae345f8cc25f5602567da) --- .../FileCarrierTechnologyParameters.java | 94 +++++++++++++++++++-- .../engine/main/ApexCommandLineArguments.java | 98 +++++++++++++++++++--- .../service/engine/runtime/impl/EngineWorker.java | 2 +- .../engineservice/EngineServiceParameters.java | 37 +++++--- 4 files changed, 201 insertions(+), 30 deletions(-) (limited to 'services/services-engine/src/main') diff --git a/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/event/impl/filecarrierplugin/FileCarrierTechnologyParameters.java b/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/event/impl/filecarrierplugin/FileCarrierTechnologyParameters.java index cbfe18016..73438cc7e 100644 --- a/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/event/impl/filecarrierplugin/FileCarrierTechnologyParameters.java +++ b/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/event/impl/filecarrierplugin/FileCarrierTechnologyParameters.java @@ -20,12 +20,14 @@ package org.onap.policy.apex.service.engine.event.impl.filecarrierplugin; +import java.io.File; + import org.onap.policy.apex.service.engine.event.impl.filecarrierplugin.consumer.ApexFileEventConsumer; import org.onap.policy.apex.service.engine.event.impl.filecarrierplugin.producer.ApexFileEventProducer; import org.onap.policy.apex.service.parameters.carriertechnology.CarrierTechnologyParameters; import org.onap.policy.common.parameters.GroupValidationResult; import org.onap.policy.common.parameters.ValidationStatus; -import org.onap.policy.common.utils.resources.ResourceUtils; +import org.onap.policy.common.utils.validation.ParameterValidationUtils; /** * This class holds the parameters that allows transport of events into and out of Apex using files and standard input @@ -52,6 +54,9 @@ public class FileCarrierTechnologyParameters extends CarrierTechnologyParameters /** The consumer plugin class for the FILE carrier technology. */ public static final String FILE_EVENT_CONSUMER_PLUGIN_CLASS = ApexFileEventConsumer.class.getCanonicalName(); + // Recurring strings + private static final String FILE_NAME_TOKEN = "fileName"; + private String fileName; private boolean standardIo = false; private boolean standardError = false; @@ -78,7 +83,7 @@ public class FileCarrierTechnologyParameters extends CarrierTechnologyParameters * @return the file name from which to read or to which to write events */ public String getFileName() { - return ResourceUtils.getFilePath4Resource(fileName); + return fileName; } /** @@ -193,9 +198,8 @@ public class FileCarrierTechnologyParameters extends CarrierTechnologyParameters public GroupValidationResult validate() { final GroupValidationResult result = super.validate(); - if (!standardIo && !standardError && (fileName == null || fileName.trim().length() == 0)) { - result.setResult("fileName", ValidationStatus.INVALID, "fileName not specified or is blank or null, " - + "it must be specified as a valid file location"); + if (!standardIo && !standardError) { + validateFileName(result); } if (standardIo || standardError) { @@ -209,4 +213,84 @@ public class FileCarrierTechnologyParameters extends CarrierTechnologyParameters return result; } + + + /** + * Validate the file name parameter. + * + * @param result the variable in which to store the result of the validation + */ + private void validateFileName(final GroupValidationResult result) { + if (!ParameterValidationUtils.validateStringParameter(fileName)) { + result.setResult(FILE_NAME_TOKEN, ValidationStatus.INVALID, + "\"" + fileName + "\" invalid, must be specified as a non-empty string"); + return; + } + + String absoluteFileName = null; + + // Resolve the file name if it is a relative file name + File theFile = new File(fileName); + if (theFile.isAbsolute()) { + absoluteFileName = fileName; + } else { + absoluteFileName = System.getProperty("APEX_RELATIVE_FILE_ROOT") + File.separator + fileName; + theFile = new File(absoluteFileName); + } + + // Check if the file exists, the file should be a regular file and should be readable + if (theFile.exists()) { + validateExistingFile(result, absoluteFileName, theFile); + } + // The path to the file should exist and should be writable + else { + validateNewFileParent(result, absoluteFileName, theFile); + } + } + + /** + * Validate an existing file is OK. + * + * @param result the result of the validation + * @param absoluteFileName the absolute file name of the file + * @param theFile the file that exists + */ + private void validateExistingFile(final GroupValidationResult result, String absoluteFileName, File theFile) { + // Check that the file is a regular file + if (!theFile.isFile()) { + result.setResult(FILE_NAME_TOKEN, ValidationStatus.INVALID, "is not a plain file"); + } + else { + fileName = absoluteFileName; + + if (!theFile.canRead()) { + result.setResult(FILE_NAME_TOKEN, ValidationStatus.INVALID, "is not readable"); + } + } + } + + /** + * Validate the parent of a new file is OK. + * + * @param result the result of the validation + * @param absoluteFileName the absolute file name of the file + * @param theFile the file that exists + */ + private void validateNewFileParent(final GroupValidationResult result, String absoluteFileName, File theFile) { + // Check that the parent of the file is a directory + if (!theFile.getParentFile().exists()) { + result.setResult(FILE_NAME_TOKEN, ValidationStatus.INVALID, "parent of file does not exist"); + } + // Check that the parent of the file is a directory + else if (!theFile.getParentFile().isDirectory()) { + result.setResult(FILE_NAME_TOKEN, ValidationStatus.INVALID, "parent of file is not directory"); + } + else { + fileName = absoluteFileName; + + if (!theFile.getParentFile().canRead()) { + result.setResult(FILE_NAME_TOKEN, ValidationStatus.INVALID, "is not readable"); + } + } + } } diff --git a/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/main/ApexCommandLineArguments.java b/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/main/ApexCommandLineArguments.java index ff0eef7f0..733327580 100644 --- a/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/main/ApexCommandLineArguments.java +++ b/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/main/ApexCommandLineArguments.java @@ -35,6 +35,7 @@ import org.apache.commons.cli.ParseException; import org.onap.policy.apex.model.basicmodel.concepts.ApexException; import org.onap.policy.apex.model.basicmodel.concepts.ApexRuntimeException; import org.onap.policy.common.utils.resources.ResourceUtils; +import org.onap.policy.common.utils.validation.ParameterValidationUtils; /** * This class reads and handles command line parameters for the Apex main program. @@ -42,6 +43,9 @@ import org.onap.policy.common.utils.resources.ResourceUtils; * @author Liam Fallon (liam.fallon@ericsson.com) */ public class ApexCommandLineArguments { + // A system property holding the root directory for relative paths in the configuration file + public static final String RELATIVE_FILE_ROOT = "APEX_RELATIVE_FILE_ROOT"; + // Recurring string constants private static final String FILE_PREAMBLE = " file \""; private static final int HELP_LINE_LENGTH = 120; @@ -52,6 +56,7 @@ public class ApexCommandLineArguments { // The command line options private String modelFilePath = null; private String configurationFilePath = null; + private String relativeFileRoot = null; /** * Construct the options for the CLI editor. @@ -66,20 +71,30 @@ public class ApexCommandLineArguments { .type(Boolean.class) .build()); options.addOption(Option.builder("v") - .longOpt("version") - .desc("outputs the version of Apex") - .required(false) - .type(Boolean.class) - .build()); + .longOpt("version") + .desc("outputs the version of Apex") + .required(false) + .type(Boolean.class) + .build()); options.addOption(Option.builder("c") - .longOpt("config-file") - .desc("the full path to the configuration file to use, the configuration file must be a Json file " - + "containing the Apex configuration parameters") - .hasArg() - .argName("CONFIG_FILE") - .required(false) - .type(String.class) - .build()); + .longOpt("config-file") + .desc("the full path to the configuration file to use, " + + "the configuration file must be a Json file " + + "containing the Apex configuration parameters") + .hasArg() + .argName("CONFIG_FILE") + .required(false) + .type(String.class) + .build()); + options.addOption(Option.builder("rfr") + .longOpt("relative-file-root") + .desc("the root file path for relative file paths specified in the Apex configuration file, " + + "defaults to the current directory from where Apex is executed") + .hasArg() + .argName(RELATIVE_FILE_ROOT) + .required(false) + .type(String.class) + .build()); options.addOption(Option.builder("m").longOpt("model-file") .desc("the full path to the model file to use, if set it overrides the model file set in the " + "configuration file").hasArg().argName("MODEL_FILE") @@ -147,6 +162,12 @@ public class ApexCommandLineArguments { setConfigurationFilePath(commandLine.getOptionValue('c')); } + if (commandLine.hasOption("rfr")) { + setRelativeFileRoot(commandLine.getOptionValue("rfr")); + } else { + setRelativeFileRoot(null); + } + if (commandLine.hasOption('m')) { setModelFilePath(commandLine.getOptionValue('m')); } @@ -165,6 +186,8 @@ public class ApexCommandLineArguments { if (checkSetModelFilePath()) { validateReadableFile("Apex model", modelFilePath); } + + validateRelativeFileRoot(); } /** @@ -228,6 +251,15 @@ public class ApexCommandLineArguments { return configurationFilePath; } + /** + * Gets the root file path for relative file paths in the configuration file. + * + * @return the root file path + */ + public String getRelativeFileRoot() { + return relativeFileRoot; + } + /** * Gets the full expanded configuration file path. * @@ -247,6 +279,29 @@ public class ApexCommandLineArguments { } + /** + * Sets the root file path for relative file paths in the configuration file. + * + * @param relativeFileRoot the configuration file path + */ + public void setRelativeFileRoot(final String relativeFileRoot) { + String relativeFileRootValue = relativeFileRoot; + + if (!ParameterValidationUtils.validateStringParameter(relativeFileRoot)) { + relativeFileRootValue = System.getProperty(RELATIVE_FILE_ROOT); + } + + if (!ParameterValidationUtils.validateStringParameter(relativeFileRootValue)) { + relativeFileRootValue = System.getProperty("user.dir"); + } + else if (!(new File(relativeFileRootValue).isAbsolute())) { + relativeFileRootValue = System.getProperty("user.dir") + File.separator + relativeFileRootValue; + } + + this.relativeFileRoot = relativeFileRootValue; + System.setProperty(RELATIVE_FILE_ROOT, relativeFileRootValue); + } + /** * Check set configuration file path. * @@ -285,4 +340,21 @@ public class ApexCommandLineArguments { throw new ApexException(fileTag + FILE_PREAMBLE + fileName + "\" is ureadable"); } } + + /** + * Validate the relative file root. + */ + private void validateRelativeFileRoot() throws ApexException { + File relativeFileRootPath = new File(relativeFileRoot); + if (!relativeFileRootPath.isDirectory()) { + throw new ApexException( + "relative file root \"" + relativeFileRoot + "\" does not exist or is not a directory"); + } + + if (!relativeFileRootPath.canWrite()) { + throw new ApexException( + "relative file root \"" + relativeFileRoot + "\" does not exist or is not a directory"); + } + } + } diff --git a/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/runtime/impl/EngineWorker.java b/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/runtime/impl/EngineWorker.java index a7d179959..a5de624d1 100644 --- a/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/runtime/impl/EngineWorker.java +++ b/services/services-engine/src/main/java/org/onap/policy/apex/service/engine/runtime/impl/EngineWorker.java @@ -659,7 +659,7 @@ final class EngineWorker implements EngineService { // Take events from the event processing queue of the worker and pass them to the engine // for processing boolean stopFlag = false; - while (!processorThread.isInterrupted() && ! stopFlag) { + while (processorThread != null && !processorThread.isInterrupted() && ! stopFlag) { ApexEvent event = null; try { event = eventProcessingQueue.take(); diff --git a/services/services-engine/src/main/java/org/onap/policy/apex/service/parameters/engineservice/EngineServiceParameters.java b/services/services-engine/src/main/java/org/onap/policy/apex/service/parameters/engineservice/EngineServiceParameters.java index 7cd75a300..35c8237c6 100644 --- a/services/services-engine/src/main/java/org/onap/policy/apex/service/parameters/engineservice/EngineServiceParameters.java +++ b/services/services-engine/src/main/java/org/onap/policy/apex/service/parameters/engineservice/EngineServiceParameters.java @@ -21,7 +21,6 @@ package org.onap.policy.apex.service.parameters.engineservice; import java.io.File; -import java.net.URL; import org.onap.policy.apex.core.engine.EngineParameters; import org.onap.policy.apex.model.basicmodel.concepts.AxArtifactKey; @@ -30,7 +29,6 @@ import org.onap.policy.apex.service.parameters.ApexParameterConstants; import org.onap.policy.common.parameters.GroupValidationResult; import org.onap.policy.common.parameters.ParameterGroup; import org.onap.policy.common.parameters.ValidationStatus; -import org.onap.policy.common.utils.resources.ResourceUtils; // @formatter:off /** @@ -218,7 +216,7 @@ public class EngineServiceParameters implements ParameterGroup { * @return the file name of the policy engine for deployment on the engine service */ public String getPolicyModelFileName() { - return ResourceUtils.getFilePath4Resource(policyModelFileName); + return policyModelFileName; } /** @@ -344,15 +342,32 @@ public class EngineServiceParameters implements ParameterGroup { return; } - // The file name can refer to a resource on the local file system or on the class - // path - final URL fileUrl = ResourceUtils.getUrl4Resource(policyModelFileName); - if (fileUrl == null) { - result.setResult(POLICY_MODEL_FILE_NAME, ValidationStatus.INVALID, "not found or is not a plain file"); + String absolutePolicyFileName = null; + + // Resolve the file name if it is a relative file name + File policyModelFile = new File(policyModelFileName); + if (policyModelFile.isAbsolute()) { + absolutePolicyFileName = policyModelFileName; + } else { + absolutePolicyFileName = System.getProperty("APEX_RELATIVE_FILE_ROOT") + File.separator + + policyModelFileName; + policyModelFile = new File(absolutePolicyFileName); + } + + // Check that the file exists + if (!policyModelFile.exists()) { + result.setResult(POLICY_MODEL_FILE_NAME, ValidationStatus.INVALID, "not found"); + } + // Check that the file is a regular file + else if (!policyModelFile.isFile()) { + result.setResult(POLICY_MODEL_FILE_NAME, ValidationStatus.INVALID, "is not a plain file"); } else { - final File policyModelFile = new File(fileUrl.getPath()); - if (!policyModelFile.isFile()) { - result.setResult(POLICY_MODEL_FILE_NAME, ValidationStatus.INVALID, "not found or is not a plain file"); + // OK, we found the file and it's OK, so reset the file name + policyModelFileName = absolutePolicyFileName; + + // Check that the file is readable + if (!policyModelFile.canRead()) { + result.setResult(POLICY_MODEL_FILE_NAME, ValidationStatus.INVALID, "is not readable"); } } } -- cgit 1.2.3-korg