diff --git a/public/java/src/org/broadinstitute/sting/commandline/Argument.java b/public/java/src/org/broadinstitute/sting/commandline/Argument.java index fa7ca9cc3..96731584b 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/Argument.java +++ b/public/java/src/org/broadinstitute/sting/commandline/Argument.java @@ -86,4 +86,40 @@ public @interface Argument { * @return Non-empty regexp for validation, blank otherwise. */ String validation() default ""; + + /** + * Hard lower bound on the allowed value for the annotated argument -- generates an exception if violated. + * Enforced only for numeric types whose values are explicitly specified on the command line. + * + * @return Hard lower bound on the allowed value for the annotated argument, or Double.NEGATIVE_INFINITY + * if there is none. + */ + double minValue() default Double.NEGATIVE_INFINITY; + + /** + * Hard upper bound on the allowed value for the annotated argument -- generates an exception if violated. + * Enforced only for numeric types whose values are explicitly specified on the command line. + * + * @return Hard upper bound on the allowed value for the annotated argument, or Double.POSITIVE_INFINITY + * if there is none. + */ + double maxValue() default Double.POSITIVE_INFINITY; + + /** + * Soft lower bound on the allowed value for the annotated argument -- generates a warning if violated. + * Enforced only for numeric types whose values are explicitly specified on the command line. + * + * @return Soft lower bound on the allowed value for the annotated argument, or Double.NEGATIVE_INFINITY + * if there is none. + */ + double minRecommendedValue() default Double.NEGATIVE_INFINITY; + + /** + * Soft upper bound on the allowed value for the annotated argument -- generates a warning if violated. + * Enforced only for numeric types whose values are explicitly specified on the command line. + * + * @return Soft upper bound on the allowed value for the annotated argument, or Double.POSITIVE_INFINITY + * if there is none. + */ + double maxRecommendedValue() default Double.POSITIVE_INFINITY; } diff --git a/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java b/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java index 5e863f4f7..aca20d5a1 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java @@ -39,6 +39,7 @@ import org.broadinstitute.sting.utils.help.HelpFormatter; import java.io.File; import java.io.IOException; +import java.lang.annotation.Annotation; import java.lang.reflect.Field; import java.util.*; @@ -46,6 +47,7 @@ import java.util.*; * A parser for Sting command-line arguments. */ public class ParsingEngine { + /** * The loaded argument sources along with their back definitions. */ @@ -376,6 +378,19 @@ public class ParsingEngine { * @param object Object into which to add arguments. */ public void loadArgumentsIntoObject( Object object ) { + loadArgumentsIntoObject(object, true); + } + + /** + * Loads a set of matched command-line arguments into the given object. + * @param object Object into which to add arguments. + * @param enforceArgumentRanges If true, check that the argument value is within the range specified + * in the corresponding Argument annotation by min/max value attributes. This + * check is only performed for numeric types, and only when a min and/or + * max value is actually defined in the annotation. It is also only performed + * for values actually specified on the command line, and not for default values. + */ + public void loadArgumentsIntoObject( Object object, boolean enforceArgumentRanges ) { List argumentSources = extractArgumentSources(object.getClass()); List dependentArguments = new ArrayList(); @@ -389,13 +404,13 @@ public class ParsingEngine { dependentArguments.add(argumentSource); continue; } - loadValueIntoObject( argumentSource, object, argumentMatches.findMatches(this,argumentSource) ); + loadValueIntoObject(argumentSource, object, argumentMatches.findMatches(this,argumentSource), enforceArgumentRanges); } for(ArgumentSource dependentArgument: dependentArguments) { MultiplexArgumentTypeDescriptor dependentDescriptor = dependentArgument.createDependentTypeDescriptor(this,object); ArgumentSource dependentSource = dependentArgument.copyWithCustomTypeDescriptor(dependentDescriptor); - loadValueIntoObject(dependentSource,object,argumentMatches.findMatches(this,dependentSource)); + loadValueIntoObject(dependentSource,object,argumentMatches.findMatches(this,dependentSource), enforceArgumentRanges); } } @@ -447,8 +462,13 @@ public class ParsingEngine { * @param argumentMatches Argument matches to load into the object. * @param source Argument source to load into the object. * @param instance Object into which to inject the value. The target might be in a container within the instance. + * @param enforceArgumentRanges If true, check that the argument value is within the range specified + * in the corresponding Argument annotation by min/max value attributes. This + * check is only performed for numeric types, and only when a min and/or + * max value is actually defined in the annotation. It is also only performed + * for values actually specified on the command line, and not for default values. */ - private void loadValueIntoObject( ArgumentSource source, Object instance, ArgumentMatches argumentMatches ) { + private void loadValueIntoObject( ArgumentSource source, Object instance, ArgumentMatches argumentMatches, boolean enforceArgumentRanges ) { // Nothing to load if( argumentMatches.size() == 0 && ! source.createsTypeDefault() ) return; @@ -461,12 +481,78 @@ public class ParsingEngine { throw new ReviewedStingException("Internal command-line parser error: unable to find a home for argument matches " + argumentMatches); for( Object target: targets ) { - Object value = (argumentMatches.size() != 0) ? source.parse(this,argumentMatches) : source.createTypeDefault(this); + Object value; + boolean usedTypeDefault = false; + if ( argumentMatches.size() != 0 ) { + value = source.parse(this,argumentMatches); + } + else { + value = source.createTypeDefault(this); + usedTypeDefault = true; + } + + // Only check argument ranges if a check was requested AND we used a value from the command line rather + // than the type default + if ( enforceArgumentRanges && ! usedTypeDefault ) { + checkArgumentRange(source, value); + } JVMUtils.setFieldValue(source.field,target,value); } } + /** + * Check the provided value against any range constraints specified in the Argument annotation + * for the corresponding field. Throw an exception if hard limits are violated, or emit a warning + * if soft limits are violated. + * + * Only checks numeric types (int, double, etc.) + * Only checks fields with an actual @Argument annotation + * Only checks manually-specified constraints (there are no default constraints). + * + * @param argumentSource The source field for the command-line argument + * @param argumentValue The value we're considering putting in that source field + */ + private void checkArgumentRange( final ArgumentSource argumentSource, final Object argumentValue ) { + // Only validate numeric types + if ( ! (argumentValue instanceof Number) ) { + return; + } + final double argumentDoubleValue = ((Number)argumentValue).doubleValue(); + + // Only validate fields with an @Argument annotation + final Annotation argumentAnnotation = argumentSource.field.getAnnotation(Argument.class); + if ( argumentAnnotation == null ) { + return; + } + + final double minValue = (Double)CommandLineUtils.getValue(argumentAnnotation, "minValue"); + final double maxValue = (Double)CommandLineUtils.getValue(argumentAnnotation, "maxValue"); + final double minRecommendedValue = (Double)CommandLineUtils.getValue(argumentAnnotation, "minRecommendedValue"); + final double maxRecommendedValue = (Double)CommandLineUtils.getValue(argumentAnnotation, "maxRecommendedValue"); + final String argumentName = (String)CommandLineUtils.getValue(argumentAnnotation, "fullName"); + + // Check hard limits first, if specified + if ( minValue != Double.NEGATIVE_INFINITY && argumentDoubleValue < minValue ) { + throw new ArgumentValueOutOfRangeException(argumentName, argumentDoubleValue, minValue, "minimum"); + } + + if ( maxValue != Double.POSITIVE_INFINITY && argumentDoubleValue > maxValue ) { + throw new ArgumentValueOutOfRangeException(argumentName, argumentDoubleValue, maxValue, "maximum"); + } + + // Then check soft limits, if specified + if ( minRecommendedValue != Double.NEGATIVE_INFINITY && argumentDoubleValue < minRecommendedValue ) { + logger.warn(String.format("WARNING: argument --%s has value %.2f, but minimum recommended value is %.2f", + argumentName, argumentDoubleValue, minRecommendedValue)); + } + + if ( maxRecommendedValue != Double.POSITIVE_INFINITY && argumentDoubleValue > maxRecommendedValue ) { + logger.warn(String.format("WARNING: argument --%s has value %.2f, but maximum recommended value is %.2f", + argumentName, argumentDoubleValue, maxRecommendedValue)); + } + } + public Collection getRodBindings() { return Collections.unmodifiableCollection(rodBindings); } @@ -654,6 +740,13 @@ class InvalidArgumentValueException extends ArgumentException { } } +class ArgumentValueOutOfRangeException extends ArgumentException { + public ArgumentValueOutOfRangeException( final String argumentName, final double argumentActualValue, + final double argumentBoundaryValue, final String argumentBoundaryType ) { + super(String.format("Argument --%s has value %.2f, but %s allowed value is %.2f", + argumentName, argumentActualValue, argumentBoundaryType, argumentBoundaryValue)); + } +} /** * An exception for values that can't be mated with any argument. diff --git a/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java b/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java index f08e04c56..29ba95963 100644 --- a/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java @@ -33,6 +33,7 @@ import org.testng.Assert; import org.broadinstitute.sting.BaseTest; import org.broadinstitute.sting.utils.exceptions.ReviewedStingException; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.File; @@ -946,4 +947,146 @@ public class ParsingEngineUnitTest extends BaseTest { FileUtils.deleteQuietly(argsFile); } } + + private class NumericRangeArgProvider { + @Argument(fullName = "intWithHardMinAndMax", minValue = 5, maxValue = 10) + public int intWithHardMinAndMax; + + @Argument(fullName = "intWithHardMin", minValue = 5) + public int intWithHardMin; + + @Argument(fullName = "intWithHardMax", maxValue = 10) + public int intWithHardMax; + + @Argument(fullName = "intWithSoftMinAndMax", minRecommendedValue = 5, maxRecommendedValue = 10) + public int intWithSoftMinAndMax; + + @Argument(fullName = "intWithSoftMin", minRecommendedValue = 5) + public int intWithSoftMin; + + @Argument(fullName = "intWithSoftMax", maxRecommendedValue = 10) + public int intWithSoftMax; + + @Argument(fullName = "intWithHardAndSoftMinAndMax", minValue = 5, minRecommendedValue = 7, maxValue = 10, maxRecommendedValue = 9) + public int intWithHardAndSoftMinAndMax; + + @Argument(fullName = "intWithHardAndSoftMin", minValue = 5, minRecommendedValue = 7) + public int intWithHardAndSoftMin; + + @Argument(fullName = "intWithHardAndSoftMax", maxValue = 10, maxRecommendedValue = 8) + public int intWithHardAndSoftMax; + + @Argument(fullName = "intWithHardMinAndMaxDefaultOutsideRange", minValue = 5, maxValue = 10) + public int intWithHardMinAndMaxDefaultOutsideRange = -1; + + @Argument(fullName = "integerWithHardMinAndMax", minValue = 5, maxValue = 10) + public Integer integerWithHardMinAndMax; + + @Argument(fullName = "byteWithHardMinAndMax", minValue = 5, maxValue = 10) + public byte byteWithHardMinAndMax; + + @Argument(fullName = "byteWithHardMin", minValue = 5) + public byte byteWithHardMin; + + @Argument(fullName = "byteWithHardMax", maxValue = 10) + public byte byteWithHardMax; + + @Argument(fullName = "doubleWithHardMinAndMax", minValue = 5.5, maxValue = 10.0) + public double doubleWithHardMinAndMax; + + @Argument(fullName = "doubleWithHardMin", minValue = 5.5) + public double doubleWithHardMin; + + @Argument(fullName = "doubleWithHardMax", maxValue = 10.0) + public double doubleWithHardMax; + } + + @DataProvider(name = "NumericRangeConstraintViolationDataProvider") + public Object[][] numericRangeConstraintViolationDataProvider() { + return new Object[][] { + { new String[]{"--intWithHardMinAndMax", "11"} }, + { new String[]{"--intWithHardMinAndMax", "4"} }, + { new String[]{"--intWithHardMin", "4"} }, + { new String[]{"--intWithHardMax", "11"} }, + { new String[]{"--intWithHardAndSoftMinAndMax", "11"} }, + { new String[]{"--intWithHardAndSoftMinAndMax", "4"} }, + { new String[]{"--intWithHardAndSoftMin", "4"} }, + { new String[]{"--intWithHardAndSoftMax", "11"} }, + { new String[]{"--intWithHardMinAndMaxDefaultOutsideRange", "11"} }, + { new String[]{"--intWithHardMinAndMaxDefaultOutsideRange", "4"} }, + { new String[]{"--integerWithHardMinAndMax", "11"} }, + { new String[]{"--integerWithHardMinAndMax", "4"} }, + { new String[]{"--byteWithHardMinAndMax", "11"} }, + { new String[]{"--byteWithHardMinAndMax", "4"} }, + { new String[]{"--byteWithHardMin", "4"} }, + { new String[]{"--byteWithHardMax", "11"} }, + { new String[]{"--doubleWithHardMinAndMax", "5.4"} }, + { new String[]{"--doubleWithHardMinAndMax", "10.1"} }, + { new String[]{"--doubleWithHardMin", "5.4"} }, + { new String[]{"--doubleWithHardMax", "10.1"} } + }; + } + + @Test(dataProvider = "NumericRangeConstraintViolationDataProvider", + expectedExceptions = ArgumentValueOutOfRangeException.class) + public void testNumericRangeWithConstraintViolation( final String[] commandLine ) { + runNumericArgumentRangeTest(commandLine); + } + + @DataProvider(name = "NumericRangeWithoutConstraintViolationDataProvider") + public Object[][] numericRangeWithoutConstraintViolationDataProvider() { + return new Object[][] { + { new String[]{"--intWithHardMinAndMax", "10"} }, + { new String[]{"--intWithHardMinAndMax", "5"} }, + { new String[]{"--intWithHardMinAndMax", "7"} }, + { new String[]{"--intWithHardMin", "11"} }, + { new String[]{"--intWithHardMax", "4"} }, + { new String[]{"--intWithSoftMinAndMax", "11"} }, + { new String[]{"--intWithSoftMinAndMax", "4"} }, + { new String[]{"--intWithSoftMin", "4"} }, + { new String[]{"--intWithSoftMax", "11"} }, + { new String[]{"--intWithHardAndSoftMinAndMax", "5"} }, + { new String[]{"--intWithHardAndSoftMinAndMax", "7"} }, + { new String[]{"--intWithHardAndSoftMinAndMax", "8"} }, + { new String[]{"--intWithHardAndSoftMinAndMax", "9"} }, + { new String[]{"--intWithHardAndSoftMinAndMax", "10"} }, + { new String[]{"--intWithHardAndSoftMin", "5"} }, + { new String[]{"--intWithHardAndSoftMin", "6"} }, + { new String[]{"--intWithHardAndSoftMin", "7"} }, + { new String[]{"--intWithHardAndSoftMax", "10"} }, + { new String[]{"--intWithHardAndSoftMax", "9"} }, + { new String[]{"--intWithHardAndSoftMax", "8"} }, + { new String[]{"--intWithHardMinAndMaxDefaultOutsideRange", "10"} }, + { new String[]{"--intWithHardMinAndMaxDefaultOutsideRange", "5"} }, + { new String[]{"--intWithHardMinAndMaxDefaultOutsideRange", "7"} }, + { new String[]{"--integerWithHardMinAndMax", "10"} }, + { new String[]{"--integerWithHardMinAndMax", "5"} }, + { new String[]{"--byteWithHardMinAndMax", "10"} }, + { new String[]{"--byteWithHardMinAndMax", "5"} }, + { new String[]{"--byteWithHardMinAndMax", "7"} }, + { new String[]{"--byteWithHardMin", "5"} }, + { new String[]{"--byteWithHardMax", "10"} }, + { new String[]{"--doubleWithHardMinAndMax", "5.5"} }, + { new String[]{"--doubleWithHardMinAndMax", "10.0"} }, + { new String[]{"--doubleWithHardMinAndMax", "7.5"} }, + { new String[]{"--doubleWithHardMin", "5.5"} }, + { new String[]{"--doubleWithHardMin", "15.5"} }, + { new String[]{"--doubleWithHardMax", "10.0"} }, + { new String[]{"--doubleWithHardMax", "7.5"} } + }; + } + + @Test(dataProvider = "NumericRangeWithoutConstraintViolationDataProvider") + public void testNumericRangeWithoutConstraintViolation( final String[] commandLine ) { + // These tests succeed if no exception is thrown, since no constraints have been violated + runNumericArgumentRangeTest(commandLine); + } + + private void runNumericArgumentRangeTest( final String[] commandLine ) { + parsingEngine.addArgumentSource(NumericRangeArgProvider.class); + parsingEngine.parse(commandLine); + + NumericRangeArgProvider argProvider = new NumericRangeArgProvider(); + parsingEngine.loadArgumentsIntoObject(argProvider); + } }