Better integrate the @ArgumentCollection into the command-line argument parser. Walkers can now specify their own @ArgumentCollections. Also cleaned up a bit of the CommandLineProgram template method pattern to minimize duplicate code.

git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@1746 348d0f76-0448-11de-a6fe-93d51630548a

java/src/org/broadinstitute/sting/gatk/CommandLineExecutable.java

@@ -47,7 +47,6 @@ import java.util.Arrays;
 public abstract class CommandLineExecutable extends CommandLineProgram {
     public Object GATKResult = null;
 
-
     /**
      * The actual engine which performs the analysis.
      */
@@ -77,19 +76,16 @@ public abstract class CommandLineExecutable extends CommandLineProgram {
     }
 
     protected Object executeGATK() {
-        GATKArgumentCollection arguments = getArgumentCollection();
-
         Walker<?,?> mWalker = GATKEngine.getWalkerByName(getAnalysisName());
 
         // load the arguments into the walkers
-        loadArgumentsIntoObject(arguments);
         loadArgumentsIntoObject(mWalker);
 
         // process any arguments that need a second pass
+        GATKArgumentCollection arguments = getArgumentCollection();
         processArguments(arguments);
 
         // set the analysis name in the argument collection
-        arguments.analysisName = getAnalysisName();
         return GATKEngine.execute(arguments, mWalker);
     }
 

java/src/org/broadinstitute/sting/gatk/GATKArgumentCollection.java

@@ -47,9 +47,6 @@ public class GATKArgumentCollection {
     public GATKArgumentCollection() {
     }
 
-    @Element(required = false)
-    public String analysisName = null;
-
     @ElementMap(entry = "analysis_argument", key = "key", attribute = true, inline = true, required = false)
     public Map<String, String> walkerArgs = new HashMap<String, String>();
 
@@ -235,9 +232,6 @@ public class GATKArgumentCollection {
         if (!other.intervals.equals(this.intervals)) {
             return false;
         }
-        if (!other.analysisName.equals(this.analysisName)) {
-            return false;
-        }
         if (!other.DBSNPFile.equals(this.DBSNPFile)) {
             return false;
         }

java/src/org/broadinstitute/sting/gatk/GenomeAnalysisEngine.java

@@ -105,6 +105,7 @@ public class GenomeAnalysisEngine {
      * Actually run the GATK with the specified walker.
      * @param args      the argument collection, where we get all our setup information from
      * @param my_walker Walker to run over the dataset.  Must not be null.
+     * @return the value of this traversal.
      */
     public Object execute(GATKArgumentCollection args, Walker<?,?> my_walker) {
         // validate our parameters
@@ -225,7 +226,7 @@ public class GenomeAnalysisEngine {
                 Utils.scareUser(String.format("Read-based traversals require a reference file but none was given"));
             microScheduler = MicroScheduler.create(my_walker, readsDataSource, referenceDataSource, rodDataSources, argCollection.numberOfThreads);
         } else {
-            Utils.scareUser(String.format("Unable to create the appropriate TraversalEngine for analysis type " + argCollection.analysisName));
+            Utils.scareUser(String.format("Unable to create the appropriate TraversalEngine for analysis type %s", WalkerManager.getWalkerName(my_walker.getClass())));
         }
 
         return microScheduler;
@@ -615,6 +616,7 @@ public class GenomeAnalysisEngine {
      * Initialize the output streams as specified by the user.
      *
      * @param walker the walker to initialize output streams for
+     * @param outputTracker the tracker supplying the initialization data.
      */
     private void initializeOutputStreams(Walker walker, OutputTracker outputTracker) {
         if( argCollection.outErrFileName != null )

java/src/org/broadinstitute/sting/gatk/io/OutputTracker.java

@@ -103,7 +103,7 @@ public abstract class OutputTracker {
                 targetValue = builder.build();
             }
 
-            JVMUtils.setField( targetField.field, walker, targetValue );
+            JVMUtils.setFieldValue( targetField.field, walker, targetValue );
         }
     }
 
@@ -171,6 +171,6 @@ public abstract class OutputTracker {
      */
     private void installStub( Walker walker, String fieldName, OutputStream outputStream ) {
         Field field = JVMUtils.findField( walker.getClass(), fieldName );
-        JVMUtils.setField( field, walker, outputStream );
+        JVMUtils.setFieldValue( field, walker, outputStream );
     }    
 }

java/src/org/broadinstitute/sting/gatk/walkers/genotyper/GenotypeCalculationModelFactory.java

@@ -1,3 +1,28 @@
+/*
+ * Copyright (c) 2009 The Broad Institute
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
 package org.broadinstitute.sting.gatk.walkers.genotyper;
 
 import static org.broadinstitute.sting.gatk.walkers.genotyper.GenotypeCalculationModel.Model.*;

java/src/org/broadinstitute/sting/gatk/walkers/genotyper/UnifiedArgumentCollection.java

@@ -1,3 +1,28 @@
+/*
+ * Copyright (c) 2009 The Broad Institute
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
 package org.broadinstitute.sting.gatk.walkers.genotyper;
 
 import org.broadinstitute.sting.utils.cmdLine.Argument;

java/src/org/broadinstitute/sting/gatk/walkers/genotyper/UnifiedGenotyper.java

@@ -1,3 +1,28 @@
+/*
+ * Copyright (c) 2009 The Broad Institute
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
 package org.broadinstitute.sting.gatk.walkers.genotyper;
 
 import org.broadinstitute.sting.gatk.GenomeAnalysisEngine;

java/src/org/broadinstitute/sting/utils/JVMUtils.java

@@ -75,7 +75,7 @@ public class JVMUtils {
      * @param instance Instance in which to set the field.
      * @param value The value to which to set the given field in the given instance.
      */
-    public static void setField( Field field, Object instance, Object value ) {
+    public static void setFieldValue( Field field, Object instance, Object value ) {
         try {
             field.setAccessible(true);
             field.set(instance, value);
@@ -85,4 +85,20 @@ public class JVMUtils {
         }
     }
 
+    /**
+     * Gets the value stored in the provided field in the given instance.
+     * @param field Field to set in the given object.
+     * @param instance Instance in which to set the field.
+     * @return Value currently stored in the given field.
+     */
+    public static Object getFieldValue( Field field, Object instance ) {
+        try {
+            field.setAccessible(true);
+            return field.get(instance);
+        }
+        catch( IllegalAccessException ex ) {
+            throw new StingException(String.format("Could not retrieve %s in instance %s",field.getName(),instance.getClass().getName()));
+        }
+    }
+
 }

java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java

@@ -124,6 +124,7 @@ public class ArgumentMatches implements Iterable<ArgumentMatch> {
 
     /**
      * Find all successful matches (a 'successful' match is one paired with a definition).
+     * @return All successful matches.
      */
     ArgumentMatches findSuccessfulMatches() {
         ArgumentMatches matches = new ArgumentMatches();
@@ -151,7 +152,6 @@ public class ArgumentMatches implements Iterable<ArgumentMatch> {
      * Merges the given argument match into the set of existing argument matches.
      * If multiple arguments are present, those arguments will end up grouped.
      * @param match The match to merge into.
-     * @return The merged match.
      */
     void mergeInto( ArgumentMatch match ) {
         boolean definitionExists = false;
@@ -228,6 +228,14 @@ class ArgumentMatch implements Iterable<ArgumentMatch> {
         indices.put(index,values );
     }
 
+    /**
+     * Return a string representation of the given argument match, for debugging purposes.
+     * @return String representation of the match.
+     */
+    public String toString() {
+        return label;
+    }
+
     /**
      * Creates an iterator that walks over each individual match at each position of a given argument.
      * @return An iterator over the individual matches in this argument.  Will not be null.

java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentSource.java

@@ -25,8 +25,6 @@
 
 package org.broadinstitute.sting.utils.cmdLine;
 
-import org.broadinstitute.sting.utils.StingException;
-
 import java.lang.reflect.Field;
 import java.util.Collection;
 import java.util.List;
@@ -96,11 +94,12 @@ public class ArgumentSource {
     }
 
     /**
-     * Injects the specified value into the selected field of the instance.
+     * Parses the specified value based on the specified type.
-     * @param targetInstance Instance into which to inject the parsed value.
+     * @param source The type of value to be parsed.
      * @param values String representation of all values passed.
+     * @return the parsed value of the object.
      */
-    public Object parse( ArgumentSource source, Object targetInstance, ArgumentMatches values ) {
+    public Object parse( ArgumentSource source, ArgumentMatches values ) {
         Object value = null;
         if( !isFlag() ) {
             ArgumentTypeDescriptor typeDescriptor = ArgumentTypeDescriptor.create( field.getType() );
@@ -128,4 +127,12 @@ public class ArgumentSource {
         Class argumentType = field.getType();
         return Collection.class.isAssignableFrom(argumentType) || field.getType().isArray();
     }
+
+    /**
+     * Gets a string representation of the argument source for debugging.
+     * @return String representation of the argument source.
+     */
+    public String toString() {
+        return clazz.getSimpleName() + ": " + field.getName();    
+    }
 }

java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java

@@ -88,7 +88,7 @@ public class ParsingEngine {
      */
     public void addArgumentSource( String sourceName, Class sourceClass ) {
         List<ArgumentDefinition> argumentsFromSource = new ArrayList<ArgumentDefinition>();
-        for( ArgumentSource argumentSource: extractArgumentSources(sourceClass,true) )
+        for( ArgumentSource argumentSource: extractArgumentSources(sourceClass) )
             argumentsFromSource.addAll( argumentSource.createArgumentDefinitions() );
         argumentDefinitions.add( new ArgumentDefinitionGroup(sourceName, argumentsFromSource) );
     }
@@ -246,31 +246,46 @@ public class ParsingEngine {
      * @param object Object into which to add arguments.
      */
     public void loadArgumentsIntoObject( Object object ) {
-        // Get a list of argument sources, not including the children of this argument.  For now, skip loading
+        List<ArgumentSource> argumentSources = extractArgumentSources(object.getClass());
-        // arguments into the object recursively.
-        List<ArgumentSource> argumentSources = extractArgumentSources( object.getClass(), false );
         for( ArgumentSource argumentSource: argumentSources )
             loadMatchesIntoObject( argumentSource, object, argumentMatches.findMatches(argumentSource) );
     }
 
     /**
-     * Loads a single argument into the object.
+     * Loads a single argument into the object and that objects children.
      * @param argumentMatches Argument matches to load into the object.
-     * @param target
+     * @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.
      */
-    private void loadMatchesIntoObject( ArgumentSource source, Object target, ArgumentMatches argumentMatches ) {
+    private void loadMatchesIntoObject( ArgumentSource source, Object instance, ArgumentMatches argumentMatches ) {
         // Nothing to load
         if( argumentMatches.size() == 0 )
             return;
 
-        if( source.clazz.isAssignableFrom(target.getClass()) ) {
+        // Target instance into which to inject the value.
-            Object value = source.parse( source, target, argumentMatches );
+        List<Object> targets = new ArrayList<Object>();
-            JVMUtils.setField( source.field, target, value );
+
+        // Check to see whether the instance itself can be the target.
+        if( source.clazz.isAssignableFrom(instance.getClass()) ) {
+            targets.add(instance);
+        }
+
+        // Check to see whether a contained class can be the target.
+        targets.addAll(getContainersMatching(instance,source.clazz));
+
+        // Abort if no home is found for the object.
+        if( targets.size() == 0 )
+            throw new StingException("Internal command-line parser error: unable to find a home for argument matches " + argumentMatches);
+
+        for( Object target: targets ) {
+            Object value = source.parse( source, argumentMatches );
+            JVMUtils.setFieldValue( source.field, target, value );
         }
     }
 
     /**
      * Prints out the help associated with these command-line argument definitions.
+     * @param applicationDetails Details about the specific GATK-based application being run.
      */
     public void printHelp( ApplicationDetails applicationDetails ) {
         new HelpFormatter().printHelp(applicationDetails,argumentDefinitions);
@@ -279,18 +294,17 @@ public class ParsingEngine {
     /**
      * Extract all the argument sources from a given object.
      * @param sourceClass class to act as sources for other arguments.
-     * @param recursive Whether to recursively look for argument collections and add their contents.
      * @return A list of sources associated with this object and its aggregated objects.
      */
-    private List<ArgumentSource> extractArgumentSources( Class sourceClass, boolean recursive ) {
+    private List<ArgumentSource> extractArgumentSources(Class sourceClass) {
         List<ArgumentSource> argumentSources = new ArrayList<ArgumentSource>();
         while( sourceClass != null ) {
             Field[] fields = sourceClass.getDeclaredFields();
             for( Field field: fields ) {
                 if( field.isAnnotationPresent(Argument.class) )
                     argumentSources.add( new ArgumentSource(sourceClass,field) );
-                if( field.isAnnotationPresent(ArgumentCollection.class) && recursive )
+                if( field.isAnnotationPresent(ArgumentCollection.class) )
-                    argumentSources.addAll( extractArgumentSources(field.getType(),recursive) );
+                    argumentSources.addAll( extractArgumentSources(field.getType()) );
             }
             sourceClass = sourceClass.getSuperclass();
         }
@@ -314,6 +328,7 @@ public class ParsingEngine {
     /**
      * Parse a short name into an ArgumentMatch.
      * @param token The token to parse.  The token should pass the isLongArgumentForm test.
+     * @param position The position of the token in question.
      * @return ArgumentMatch associated with this token, or null if no match exists.
      */    
     private ArgumentMatch parseArgument( String token, int position ) {
@@ -328,6 +343,24 @@ public class ParsingEngine {
         // No parse results found.
         return null;
     }
+
+    /**
+     * Gets a list of the container instances of the given type stored within the given target.
+     * @param target Class holding the container.
+     * @param type Container type.
+     * @return A list of containers matching the given type.
+     */
+    private List<Object> getContainersMatching(Object target, Class<?> type) {
+        List<Object> containers = new ArrayList<Object>();
+
+        Field[] fields = target.getClass().getDeclaredFields();
+        for( Field field: fields ) {
+            if( field.isAnnotationPresent(ArgumentCollection.class) && type.isAssignableFrom(field.getType()) )
+                containers.add(JVMUtils.getFieldValue(field,target));
+        }
+
+        return containers;
+    }
 }
 
 /**

java/test/org/broadinstitute/sting/gatk/GATKArgumentCollectionTest.java

@@ -1,7 +1,6 @@
 package org.broadinstitute.sting.gatk;
 
 import org.broadinstitute.sting.BaseTest;
-import org.broadinstitute.sting.gatk.GATKArgumentCollection;
 import org.junit.After;
 import static org.junit.Assert.fail;
 import org.junit.Before;
@@ -83,7 +82,6 @@ public class GATKArgumentCollectionTest extends BaseTest {
         collect.maximumEngineIterations = -1;
         collect.strictnessLevel = SAMFileReader.ValidationStringency.STRICT;
         collect.referenceFile = new File("referenceFile".toLowerCase());
-        collect.analysisName = "analysisName".toLowerCase();
         collect.DBSNPFile = "DBSNPFile".toLowerCase();
         collect.HAPMAPFile = "HAPMAPFile".toLowerCase();
         collect.HAPMAPChipFile = "HAPMAPChipFile".toLowerCase();

java/test/org/broadinstitute/sting/utils/cmdLine/ParsingEngineTest.java

@@ -201,7 +201,7 @@ public class ParsingEngineTest extends BaseTest {
         Assert.assertEquals("Enum value is not correct", TestEnum.THREE, argProvider.testEnum);
     }
 
-    public enum TestEnum { ONE, TWO, THREE };
+    public enum TestEnum { ONE, TWO, THREE }
 
     private class EnumArgProvider {
         @Argument(fullName="test_enum",shortName="ti",doc="test enum",required=false)
@@ -562,4 +562,35 @@ public class ParsingEngineTest extends BaseTest {
         @Argument(doc="value",validation="\\d+")
         Integer value;
     }
+
+    @Test
+    public void argumentCollectionTest() {
+        String[] commandLine = new String[] { "--value", "5" };
+
+        parsingEngine.addArgumentSource( ArgumentCollectionProvider.class );
+        parsingEngine.parse( commandLine );
+        parsingEngine.validate();
+
+        ArgumentCollectionProvider argProvider = new ArgumentCollectionProvider();
+        parsingEngine.loadArgumentsIntoObject(argProvider);
+
+        Assert.assertEquals("Argument is not correctly initialized", 5, argProvider.rap.value.intValue() );
+    }
+
+    private class ArgumentCollectionProvider {
+        @ArgumentCollection
+        RequiredArgProvider rap = new RequiredArgProvider();
+    }
+
+    @Test(expected=StingException.class)
+    public void multipleArgumentCollectionTest() {
+        parsingEngine.addArgumentSource( MultipleArgumentCollectionProvider.class );
+    }
+
+    private class MultipleArgumentCollectionProvider {
+        @ArgumentCollection
+        RequiredArgProvider rap1 = new RequiredArgProvider();
+        @ArgumentCollection
+        RequiredArgProvider rap2 = new RequiredArgProvider();
+    }
 }