From 40e06f9afb871d0e269261c79e626743105fe383 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Thu, 11 Aug 2011 08:58:30 -0400 Subject: [PATCH] Fixed broken RodBinding defaults. -- Verified now to be correct at runtime -- UnitTest covers this -- createTypeDefault now takes a Type, not a Class, so that parameterized classes can have their parameter fetched in the defaults. --- .../sting/commandline/ArgumentSource.java | 2 +- .../commandline/ArgumentTypeDescriptor.java | 10 ++++--- .../sting/commandline/ParsingEngine.java | 2 +- .../OutputStreamArgumentTypeDescriptor.java | 4 +-- .../SAMFileWriterArgumentTypeDescriptor.java | 2 +- .../VCFWriterArgumentTypeDescriptor.java | 2 +- .../commandline/ParsingEngineUnitTest.java | 28 +++++++++++++++---- 7 files changed, 34 insertions(+), 16 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java b/public/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java index c3402bd0a..e0e2ac378 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java @@ -185,7 +185,7 @@ public class ArgumentSource { * @return A default value for the given type. */ public Object createTypeDefault(ParsingEngine parsingEngine) { - return typeDescriptor.createTypeDefault(parsingEngine,this,field.getType()); + return typeDescriptor.createTypeDefault(parsingEngine,this,field.getGenericType()); } /** diff --git a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java index 64c00a726..d1d4ff914 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java @@ -96,12 +96,13 @@ public abstract class ArgumentTypeDescriptor { /** * Generates a default for the given type. + * * @param parsingEngine the parsing engine used to validate this argument type descriptor. * @param source Source of the command-line argument. * @param type Type of value to create, in case the command-line argument system wants influence. * @return A default value for the given type. */ - public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source,Class type) { throw new UnsupportedOperationException("Unable to create default for type " + getClass()); } + public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source, Type type) { throw new UnsupportedOperationException("Unable to create default for type " + getClass()); } /** * Given the given argument source and attributes, synthesize argument definitions for command-line arguments. @@ -323,8 +324,9 @@ class RodBindingArgumentTypeDescriptor extends ArgumentTypeDescriptor { public boolean createsTypeDefault(ArgumentSource source) { return ! source.isRequired(); } @Override - public Object createTypeDefault(ParsingEngine parsingEngine, ArgumentSource source, Class type) { - return RodBinding.makeUnbound((Class)type); + public Object createTypeDefault(ParsingEngine parsingEngine, ArgumentSource source, Type type) { + Class parameterType = getParameterizedTypeClass(type); + return RodBinding.makeUnbound((Class)parameterType); } @Override @@ -646,7 +648,7 @@ class MultiplexArgumentTypeDescriptor extends ArgumentTypeDescriptor { } @Override - public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source,Class type) { + public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source, Type type) { if(multiplexer == null || multiplexedIds == null) throw new ReviewedStingException("No multiplexed ids available"); diff --git a/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java b/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java index 9b543142b..fbf8c6516 100755 --- a/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java @@ -364,7 +364,7 @@ public class ParsingEngine { */ private void loadValueIntoObject( ArgumentSource source, Object instance, ArgumentMatches argumentMatches ) { // Nothing to load - if( argumentMatches.size() == 0 && !(source.createsTypeDefault() && source.isRequired())) + if( argumentMatches.size() == 0 && ! source.createsTypeDefault() ) return; // Target instance into which to inject the value. diff --git a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/OutputStreamArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/OutputStreamArgumentTypeDescriptor.java index b70d9eeec..da4eb3955 100644 --- a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/OutputStreamArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/OutputStreamArgumentTypeDescriptor.java @@ -75,12 +75,12 @@ public class OutputStreamArgumentTypeDescriptor extends ArgumentTypeDescriptor { } @Override - public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source,Class type) { + public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source, Type type) { if(!source.isRequired()) throw new ReviewedStingException("BUG: tried to create type default for argument type descriptor that can't support a type default."); OutputStreamStub stub = new OutputStreamStub(defaultOutputStream); engine.addOutput(stub); - return createInstanceOfClass(type,stub); + return createInstanceOfClass((Class)type,stub); } @Override diff --git a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java index 8d67a837e..43ec934ed 100644 --- a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java @@ -99,7 +99,7 @@ public class SAMFileWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor } @Override - public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source,Class type) { + public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source, Type type) { if(!source.isRequired()) throw new ReviewedStingException("BUG: tried to create type default for argument type descriptor that can't support a type default."); SAMFileWriterStub stub = new SAMFileWriterStub(engine,defaultOutputStream); diff --git a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/VCFWriterArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/VCFWriterArgumentTypeDescriptor.java index 6e5499339..98026554b 100644 --- a/public/java/src/org/broadinstitute/sting/gatk/io/stubs/VCFWriterArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/gatk/io/stubs/VCFWriterArgumentTypeDescriptor.java @@ -114,7 +114,7 @@ public class VCFWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor { } @Override - public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source,Class type) { + public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source, Type type) { if(!source.isRequired()) throw new ReviewedStingException("BUG: tried to create type default for argument type descriptor that can't support a type default."); VCFWriterStub stub = new VCFWriterStub(engine, defaultOutputStream, false, argumentSources, false, false); diff --git a/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java b/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java index 366401ad6..79e9172dd 100755 --- a/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/commandline/ParsingEngineUnitTest.java @@ -632,8 +632,8 @@ public class ParsingEngineUnitTest extends BaseTest { // -------------------------------------------------------------------------------- private class SingleRodBindingArgProvider { - @Input(fullName="binding", shortName="V", required=false) - public RodBinding binding = RodBinding.makeUnbound(Feature.class); + @Input(fullName="binding", shortName="V", required=true) + public RodBinding binding; } @Test @@ -656,7 +656,7 @@ public class ParsingEngineUnitTest extends BaseTest { private class ShortNameOnlyRodBindingArgProvider { @Input(shortName="short", required=false) - public RodBinding binding = RodBinding.makeUnbound(Feature.class); + public RodBinding binding; // = RodBinding.makeUnbound(Feature.class); } @Test @@ -677,22 +677,38 @@ public class ParsingEngineUnitTest extends BaseTest { Assert.assertEquals(argProvider.binding.getTags().getPositionalTags().size(), 1, "Tags aren't correctly set"); } + private class OptionalRodBindingArgProvider { + @Input(fullName="binding", shortName="V", required=false) + public RodBinding binding; + + @Input(fullName="bindingNull", shortName="VN", required=false) + public RodBinding bindingNull = null; + } + @Test - public void unbasicRodBindingArgumentTest() { + public void optionalRodBindingArgumentTest() { final String[] commandLine = new String[] {}; - parsingEngine.addArgumentSource( SingleRodBindingArgProvider.class ); + parsingEngine.addArgumentSource( OptionalRodBindingArgProvider.class ); parsingEngine.parse( commandLine ); parsingEngine.validate(); - SingleRodBindingArgProvider argProvider = new SingleRodBindingArgProvider(); + OptionalRodBindingArgProvider argProvider = new OptionalRodBindingArgProvider(); parsingEngine.loadArgumentsIntoObject( argProvider ); + Assert.assertNotNull(argProvider.binding, "Default value not applied corrected to RodBinding"); Assert.assertEquals(argProvider.binding.getName(), RodBinding.UNBOUND_VARIABLE_NAME, "Name isn't set properly"); Assert.assertEquals(argProvider.binding.getSource(), RodBinding.UNBOUND_SOURCE, "Source isn't set to its expected value"); Assert.assertEquals(argProvider.binding.getType(), Feature.class, "Type isn't set to its expected value"); Assert.assertEquals(argProvider.binding.isBound(), false, "Bound() isn't returning its expected value"); Assert.assertEquals(argProvider.binding.getTags().getPositionalTags().size(), 0, "Tags aren't correctly set"); + + Assert.assertNotNull(argProvider.bindingNull, "Default value not applied corrected to RodBinding"); + Assert.assertEquals(argProvider.bindingNull.getName(), RodBinding.UNBOUND_VARIABLE_NAME, "Name isn't set properly"); + Assert.assertEquals(argProvider.bindingNull.getSource(), RodBinding.UNBOUND_SOURCE, "Source isn't set to its expected value"); + Assert.assertEquals(argProvider.bindingNull.getType(), VariantContext.class, "Type isn't set to its expected value"); + Assert.assertEquals(argProvider.bindingNull.isBound(), false, "Bound() isn't returning its expected value"); + Assert.assertEquals(argProvider.bindingNull.getTags().getPositionalTags().size(), 0, "Tags aren't correctly set"); } @Test(expectedExceptions = UserException.class)