From d014c7faf984a866753263f0a4031848a888f35f Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 28 Nov 2011 13:35:14 -0500 Subject: [PATCH 1/3] Queue now properly escapes all shell arguments in generated shell scripts This has implications for both Qscript authors and CommandLineFunction authors. Qscript authors: You no longer need to (and in fact must not) manually escape String values to avoid interpretation by the shell when setting up Walker parameters. Queue will safely escape all of your Strings for you so that they'll be interpreted literally. Eg., Old way: filterSNPs.filterExpression = List("\"QD<2.0\"", "\"MQ<40.0\"", "\"HaplotypeScore>13.0\"") New way: filterSNPs.filterExpression = List("QD<2.0", "MQ<40.0", "HaplotypeScore>13.0") CommandLineFunction authors: If you're writing a one-off CommandLineFunction in a Qscript and don't really care about quoting issues, just keep doing things the direct, simple way: def commandLine = "cat %s | grep -v \"#\" > %s".format(files, out) If you're writing a CommandLineFunction that will become part of Queue and will be used by other QScripts, however, it's advisable to do things the newer, safer way, ie.: When you construct your commandLine, you should do so ONLY using the API methods required(), optional(), conditional(), and repeat(). These will manage quoting and whitespace separation for you, so you shouldn't insert quotes/extraneous whitespace in your Strings. By default you get both (quoting and whitespace separation), but you can disable either of these via parameters. Eg., override def commandLine = super.commandLine + required("eff") + conditional(verbose, "-v") + optional("-c", config) + required("-i", "vcf") + required("-o", "vcf") + required(genomeVersion) + required(inVcf) + required(">", escape=false) + // This will be shell-interpreted required(outVcf) I've ported the Picard/Samtools/SnpEff CommandLineFunction classes to the new system, so you'll get free shell escaping when you use those in Qscripts just like with walkers. --- .../gatk/ArgumentDefinitionField.java | 20 +- .../gatk/GATKExtensionsGenerator.java | 3 + .../queue/extensions/gatk/RodBindField.java | 129 ----------- .../queue/qscripts/GATKResourcesBundle.scala | 2 +- .../MethodsDevelopmentCallingPipeline.scala | 4 +- .../examples/ExampleUnifiedGenotyper.scala | 2 +- .../sting/queue/extensions/gatk/RodBind.scala | 17 +- .../queue/extensions/gatk/TaggedFile.scala | 18 +- .../picard/AddOrReplaceReadGroups.scala | 14 +- .../extensions/picard/MarkDuplicates.scala | 10 +- .../extensions/picard/MergeSamFiles.scala | 8 +- .../extensions/picard/PicardBamFunction.scala | 19 +- .../queue/extensions/picard/ReorderSam.scala | 6 +- .../queue/extensions/picard/RevertSam.scala | 12 +- .../queue/extensions/picard/SamToFastq.scala | 26 +-- .../extensions/picard/ValidateSamFile.scala | 14 +- .../samtools/SamtoolsIndexFunction.scala | 5 +- .../samtools/SamtoolsMergeFunction.scala | 8 +- .../queue/extensions/snpeff/SnpEff.scala | 17 +- .../queue/function/CommandLineFunction.scala | 209 ++++++++++++++---- .../function/JavaCommandLineFunction.scala | 34 +-- .../sting/queue/util/ShellUtils.scala | 36 +++ .../CommandLineFunctionUnitTest.scala | 171 ++++++++++++++ .../sting/queue/util/ShellUtilsUnitTest.scala | 57 +++++ 24 files changed, 545 insertions(+), 296 deletions(-) delete mode 100644 public/java/src/org/broadinstitute/sting/queue/extensions/gatk/RodBindField.java create mode 100644 public/scala/src/org/broadinstitute/sting/queue/util/ShellUtils.scala create mode 100644 public/scala/test/org/broadinstitute/sting/queue/function/CommandLineFunctionUnitTest.scala create mode 100644 public/scala/test/org/broadinstitute/sting/queue/util/ShellUtilsUnitTest.scala diff --git a/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/ArgumentDefinitionField.java b/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/ArgumentDefinitionField.java index cb5bad4ae..cdfc329e8 100644 --- a/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/ArgumentDefinitionField.java +++ b/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/ArgumentDefinitionField.java @@ -83,10 +83,10 @@ public abstract class ArgumentDefinitionField extends ArgumentField { getShortFieldSetter()); } - protected static final String REQUIRED_TEMPLATE = " + \" %1$s \" + %2$s.format(%3$s)"; - protected static final String REPEAT_TEMPLATE = " + repeat(\" %1$s \", %3$s, format=formatValue(%2$s))"; - protected static final String OPTIONAL_TEMPLATE = " + optional(\" %1$s \", %3$s, format=formatValue(%2$s))"; - protected static final String FLAG_TEMPLATE = " + (if (%3$s) \" %1$s\" else \"\")"; + protected static final String REQUIRED_TEMPLATE = " + required(\"%1$s\", %3$s, spaceSeparated=true, escape=true, format=%2$s)"; + protected static final String REPEAT_TEMPLATE = " + repeat(\"%1$s\", %3$s, spaceSeparated=true, escape=true, format=%2$s)"; + protected static final String OPTIONAL_TEMPLATE = " + optional(\"%1$s\", %3$s, spaceSeparated=true, escape=true, format=%2$s)"; + protected static final String FLAG_TEMPLATE = " + conditional(%3$s, \"%1$s\", escape=true, format=%2$s)"; public final String getCommandLineAddition() { return String.format(getCommandLineTemplate(), getCommandLineParam(), getCommandLineFormat(), getFieldName()); @@ -136,7 +136,7 @@ public abstract class ArgumentDefinitionField extends ArgumentField { new IntervalStringArgumentField(argumentDefinition)); // ROD Bindings are set by the RodBindField - } else if (RodBindField.ROD_BIND_FIELD.equals(argumentDefinition.fullName) && argumentDefinition.ioType == ArgumentIOType.INPUT) { + } else if (RodBindArgumentField.ROD_BIND_FIELD.equals(argumentDefinition.fullName) && argumentDefinition.ioType == ArgumentIOType.INPUT) { // TODO: Once everyone is using @Allows and @Requires correctly, we can stop blindly allowing Triplets return Arrays.asList(new RodBindArgumentField(argumentDefinition), new InputIndexesArgumentField(argumentDefinition, Tribble.STANDARD_INDEX_EXTENSION)); //return Collections.emptyList(); @@ -337,6 +337,8 @@ public abstract class ArgumentDefinitionField extends ArgumentField { // Allows the user to specify the track name, track type, and the file. public static class RodBindArgumentField extends ArgumentDefinitionField { + public static final String ROD_BIND_FIELD = "rodBind"; + public RodBindArgumentField(ArgumentDefinition argumentDefinition) { super(argumentDefinition); } @@ -344,7 +346,7 @@ public abstract class ArgumentDefinitionField extends ArgumentField { @Override protected String getFieldType() { return "List[RodBind]"; } @Override protected String getDefaultValue() { return "Nil"; } @Override protected String getCommandLineTemplate() { - return " + repeat(\"\", %3$s, format=RodBind.formatCommandLine(\"%1$s\"))"; + return " + repeat(\"%1$s\", %3$s, formatPrefix=RodBind.formatCommandLineParameter, spaceSeparated=true, escape=true, format=%2$s)"; } } @@ -358,11 +360,11 @@ public abstract class ArgumentDefinitionField extends ArgumentField { @Override protected String getDefaultValue() { return argumentDefinition.isMultiValued ? "Nil" : "_"; } @Override protected String getCommandLineTemplate() { if (argumentDefinition.isMultiValued) { - return " + repeat(\"\", %3$s, format=TaggedFile.formatCommandLine(\"%1$s\"))"; + return " + repeat(\"%1$s\", %3$s, formatPrefix=TaggedFile.formatCommandLineParameter, spaceSeparated=true, escape=true, format=%2$s)"; } else if (!argumentDefinition.required) { - return " + optional(\"\", %3$s, format=TaggedFile.formatCommandLine(\"%1$s\"))"; + return " + optional(TaggedFile.formatCommandLineParameter(\"%1$s\", %3$s), %3$s, spaceSeparated=true, escape=true, format=%2$s)"; } else { - return " + TaggedFile.formatCommandLine(\"%1$s\")(\"\", %3$s, \"\")"; + return " + required(TaggedFile.formatCommandLineParameter(\"%1$s\", %3$s), %3$s, spaceSeparated=true, escape=true, format=%2$s)"; } } } diff --git a/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/GATKExtensionsGenerator.java b/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/GATKExtensionsGenerator.java index 9578eda84..67010c4d5 100644 --- a/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/GATKExtensionsGenerator.java +++ b/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/GATKExtensionsGenerator.java @@ -374,6 +374,9 @@ public class GATKExtensionsGenerator extends CommandLineProgram { if (isGather) importSet.add("import org.broadinstitute.sting.commandline.Gather"); + // Needed for ShellUtils.escapeShellArgument() + importSet.add("import org.broadinstitute.sting.queue.util.ShellUtils"); + // Sort the imports so that the are always in the same order. List sortedImports = new ArrayList(importSet); Collections.sort(sortedImports); diff --git a/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/RodBindField.java b/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/RodBindField.java deleted file mode 100644 index baf083575..000000000 --- a/public/java/src/org/broadinstitute/sting/queue/extensions/gatk/RodBindField.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Copyright (c) 2010, 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.queue.extensions.gatk; - -import org.broadinstitute.sting.commandline.Input; -import org.broadinstitute.sting.gatk.WalkerManager; -import org.broadinstitute.sting.gatk.refdata.tracks.RMDTrackBuilder; -import org.broadinstitute.sting.gatk.walkers.RMD; -import org.broadinstitute.sting.gatk.walkers.Walker; - -import java.io.File; -import java.lang.annotation.Annotation; -import java.util.ArrayList; -import java.util.List; - -/** - * Allows user to specify the rod file but locks in the track name and the track type. - */ -public class RodBindField extends ArgumentField { - public static final String ROD_BIND_FIELD = "rodBind"; - - private final String trackName; - private final String typeName; - private final List relatedFields; - private final boolean isRequired; - - public RodBindField(String trackName, String typeName, List relatedFields, boolean isRequired) { - this.trackName = trackName; - this.typeName = typeName; - this.relatedFields = relatedFields; - this.isRequired = isRequired; - } - - @SuppressWarnings("unchecked") - @Override protected Class getAnnotationIOClass() { return Input.class; } - @Override protected Class getInnerType() { return File.class; } - @Override protected String getFullName() { return escape(getRawFieldName()); } - @Override protected String getFieldType() { return "File"; } - @Override protected String getDefaultValue() { return "_"; } - @Override protected String getRawFieldName() { return this.trackName + this.typeName; } - @Override protected String getDoc() { return escape(this.typeName + " " + this.trackName); } - @Override protected boolean isRequired() { return this.isRequired; } - - @Override public String getCommandLineAddition() { - // TODO: Stop allowing the generic "rodBind" triplets to satisfy the requirement after @Requires are fixed. - return String.format(" + optional(\" -B:%s,%s \", %s)", - /* - return String.format(this.useOption() - ? " + optional(\" -B:%s,%s \", %s)" - : " + \" -B:%s,%s \" + %s", - */ - this.trackName, this.typeName, getFieldName()); - } - - private boolean useOption() { - return !this.isRequired || (relatedFields.size() > 1); - } - - @Override protected String getExclusiveOf() { - StringBuilder exclusiveOf = new StringBuilder(); - // TODO: Stop allowing the generic "rodBind" triplets to satisfy the requirement after @Requires are fixed. - if (this.isRequired) - exclusiveOf.append(ROD_BIND_FIELD); - for (RodBindField relatedField: relatedFields) - if (relatedField != this) { - if (exclusiveOf.length() > 0) - exclusiveOf.append(","); - exclusiveOf.append(relatedField.getFieldName()); - } - return exclusiveOf.toString(); - } -// -// public static List getRodArguments(Class walkerClass, RMDTrackBuilder trackBuilder) { -// List argumentFields = new ArrayList(); -// -// List requires = WalkerManager.getRequiredMetaData(walkerClass); -// List allows = WalkerManager.getAllowsMetaData(walkerClass); -// -// for (RMD required: requires) { -// List fields = new ArrayList(); -// String trackName = required.name(); -// if ("*".equals(trackName)) { -// // TODO: Add the field triplet for name=* after @Allows and @Requires are fixed on walkers -// //fields.add(new RodBindArgumentField(argumentDefinition, true)); -// } else { -// for (String typeName: trackBuilder.getFeatureManager().getTrackRecordTypeNames(required.type())) -// fields.add(new RodBindField(trackName, typeName, fields, true)); -// } -// argumentFields.addAll(fields); -// } -// -// for (RMD allowed: allows) { -// List fields = new ArrayList(); -// String trackName = allowed.name(); -// if ("*".equals(trackName)) { -// // TODO: Add the field triplet for name=* after @Allows and @Requires are fixed on walkers -// //fields.add(new RodBindArgumentField(argumentDefinition, false)); -// } else { -// for (String typeName: trackBuilder.getFeatureManager().getTrackRecordTypeNames(allowed.type())) -// fields.add(new RodBindField(trackName, typeName, fields, true)); -// } -// argumentFields.addAll(fields); -// } -// -// return argumentFields; -// } -} diff --git a/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/GATKResourcesBundle.scala b/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/GATKResourcesBundle.scala index 036a77b58..8c9063c29 100755 --- a/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/GATKResourcesBundle.scala +++ b/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/GATKResourcesBundle.scala @@ -314,7 +314,7 @@ class GATKResourcesBundle extends QScript { class MakeDBSNP129(@Input dbsnp: File, @Input ref: File, @Output dbsnp129: File) extends SelectVariants with UNIVERSAL_GATK_ARGS { this.variant = dbsnp - this.select ++= List("\"dbSNPBuildID <= 129\"") + this.select ++= List("dbSNPBuildID <= 129") this.reference_sequence = ref this.out = dbsnp129 } diff --git a/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/MethodsDevelopmentCallingPipeline.scala b/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/MethodsDevelopmentCallingPipeline.scala index da02c8ac5..c06601a2d 100755 --- a/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/MethodsDevelopmentCallingPipeline.scala +++ b/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/MethodsDevelopmentCallingPipeline.scala @@ -248,10 +248,10 @@ class MethodsDevelopmentCallingPipeline extends QScript { this.V = t.rawIndelVCF this.out = t.filteredIndelVCF this.filterName ++= List("IndelQD", "IndelReadPosRankSum", "IndelFS") - this.filterExpression ++= List("\"QD < 2.0\"", "\"ReadPosRankSum < -20.0\"", "\"FS > 200.0\"") + this.filterExpression ++= List("QD < 2.0", "ReadPosRankSum < -20.0", "FS > 200.0") if (t.nSamples >= 10) { this.filterName ++= List("IndelInbreedingCoeff") - this.filterExpression ++= List("\"InbreedingCoeff < -0.8\"") + this.filterExpression ++= List("InbreedingCoeff < -0.8") } this.analysisName = t.name + "_VF" this.jobName = queueLogDir + t.name + ".indelfilter" diff --git a/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/examples/ExampleUnifiedGenotyper.scala b/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/examples/ExampleUnifiedGenotyper.scala index 9bddfd97c..2f4aea755 100644 --- a/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/examples/ExampleUnifiedGenotyper.scala +++ b/public/scala/qscript/org/broadinstitute/sting/queue/qscripts/examples/ExampleUnifiedGenotyper.scala @@ -62,7 +62,7 @@ class ExampleUnifiedGenotyper extends QScript { variantFilter.variant = genotyper.out variantFilter.out = swapExt(qscript.bamFile, "bam", "filtered.vcf") variantFilter.filterName = filterNames - variantFilter.filterExpression = filterExpressions.map("\"" + _ + "\"") + variantFilter.filterExpression = filterExpressions evalFiltered.eval :+= variantFilter.out evalFiltered.out = swapExt(variantFilter.out, "vcf", "eval") diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/RodBind.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/RodBind.scala index 9af4d9bcf..deb83bf5a 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/RodBind.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/RodBind.scala @@ -28,21 +28,14 @@ object RodBind { def apply(trackName: String, trackType: String, file: File, tag: String) = new RodBind(trackName, trackType, file, tag) def apply(trackName: String, trackType: String, file: File) = new RodBind(trackName, trackType, file, null) - /** - * Formats the rod binding on the command line. - * Used for optional and repeat. - * @param cmdLineParam command line parameter, ex: -B - * @param prefix unused - * @param value RodBind to add. - * @param suffix unused - * @return The command line addition. - */ - def formatCommandLine(cmdLineParam: String)(prefix: String, value: Any, suffix: String) = { + def formatCommandLineParameter( cmdLineParam: String, value: Any ) = { value match { case rodBind: RodBind if (rodBind.tag != null) => - " %s:%s,%s,%s %s".format(cmdLineParam, rodBind.trackName, rodBind.trackType, rodBind.tag, rodBind.getPath) + "%s:%s,%s,%s".format(cmdLineParam, rodBind.trackName, rodBind.trackType, rodBind.tag) case rodBind: RodBind => - " %s:%s,%s %s".format(cmdLineParam, rodBind.trackName, rodBind.trackType, rodBind.getPath) + "%s:%s,%s".format(cmdLineParam, rodBind.trackName, rodBind.trackType) + case x => + "" } } } diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/TaggedFile.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/TaggedFile.scala index 295199993..940985f92 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/TaggedFile.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/TaggedFile.scala @@ -2,6 +2,7 @@ package org.broadinstitute.sting.queue.extensions.gatk import java.io.File import org.broadinstitute.sting.utils.io.FileExtension +import org.broadinstitute.sting.queue.util.ShellUtils /** * Used to provide tagged -I input_file arguments to the GATK. @@ -19,21 +20,14 @@ object TaggedFile { def apply(path: String, tag: String) = new TaggedFile(path, tag) def apply(file: File, tag: String) = new TaggedFile(file, tag) - /** - * Formats the rod binding on the command line. - * Used for optional and repeat. - * @param cmdLineParam command line parameter, ex: -I - * @param prefix unused - * @param value TaggedFile to add. - * @param suffix unused - * @return The command line addition. - */ - def formatCommandLine(cmdLineParam: String)(prefix: String, value: Any, suffix: String) = { + def formatCommandLineParameter(cmdLineParam: String, value: Any) = { value match { case taggedFile: TaggedFile if (taggedFile.tag != null) => - " %s:%s %s".format(cmdLineParam, taggedFile.tag, taggedFile.getPath) + "%s:%s".format(cmdLineParam, taggedFile.tag) case file: File => - " %s %s".format(cmdLineParam, file.getPath) + cmdLineParam + case x => + "" } } } diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/AddOrReplaceReadGroups.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/AddOrReplaceReadGroups.scala index 5456ed02c..93735e4ac 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/AddOrReplaceReadGroups.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/AddOrReplaceReadGroups.scala @@ -55,11 +55,11 @@ class AddOrReplaceReadGroups extends org.broadinstitute.sting.queue.function.Jav override def outputBam = output this.createIndex = Some(true) override def commandLine = super.commandLine + - " RGID=" + RGID + - " RGLB=" + RGLB + - " RGPL=" + RGPL + - " RGPU=" + RGPU + - " RGSM=" + RGSM + - conditionalParameter(RGCN != null && !RGCN.isEmpty, " RGCN=" + RGCN) + - conditionalParameter(RGDS != null && !RGDS.isEmpty, " RGDS=" + RGDS) + required("RGID=" + RGID) + + required("RGLB=" + RGLB) + + required("RGPL=" + RGPL) + + required("RGPU=" + RGPU) + + required("RGSM=" + RGSM) + + conditional(RGCN != null && !RGCN.isEmpty, "RGCN=" + RGCN) + + conditional(RGDS != null && !RGDS.isEmpty, "RGDS=" + RGDS) } \ No newline at end of file diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MarkDuplicates.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MarkDuplicates.scala index d44d5e004..d73c556af 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MarkDuplicates.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MarkDuplicates.scala @@ -47,10 +47,8 @@ class MarkDuplicates extends org.broadinstitute.sting.queue.function.JavaCommand this.sortOrder = null this.createIndex = Some(true) override def commandLine = super.commandLine + - " M=" + metrics + - conditionalParameter(REMOVE_DUPLICATES, " REMOVE_DUPLICATES=true") + - conditionalParameter(MAX_FILE_HANDLES_FOR_READ_ENDS_MAP > 0, " MAX_FILE_HANDLES_FOR_READ_ENDS_MAP=" + MAX_FILE_HANDLES_FOR_READ_ENDS_MAP.toString) + - conditionalParameter(SORTING_COLLECTION_SIZE_RATIO > 0, " SORTING_COLLECTION_SIZE_RATIO=" + SORTING_COLLECTION_SIZE_RATIO.toString) - - + required("M=" + metrics) + + conditional(REMOVE_DUPLICATES, "REMOVE_DUPLICATES=true") + + conditional(MAX_FILE_HANDLES_FOR_READ_ENDS_MAP > 0, "MAX_FILE_HANDLES_FOR_READ_ENDS_MAP=" + MAX_FILE_HANDLES_FOR_READ_ENDS_MAP.toString) + + conditional(SORTING_COLLECTION_SIZE_RATIO > 0, "SORTING_COLLECTION_SIZE_RATIO=" + SORTING_COLLECTION_SIZE_RATIO.toString) } \ No newline at end of file diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MergeSamFiles.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MergeSamFiles.scala index fd107890e..036932cc6 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MergeSamFiles.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/MergeSamFiles.scala @@ -44,9 +44,7 @@ class MergeSamFiles extends org.broadinstitute.sting.queue.function.JavaCommandL override def outputBam = output this.createIndex = Some(true) override def commandLine = super.commandLine + - conditionalParameter(MERGE_SEQUENCE_DICTIONARIES, " MERGE_SEQUENCE_DICTIONARIES=true") + - conditionalParameter(USE_THREADING, " USE_THREADING=true") + - conditionalParameter(COMMENT != null && !COMMENT.isEmpty, " COMMENT=" + COMMENT) - - + conditional(MERGE_SEQUENCE_DICTIONARIES, "MERGE_SEQUENCE_DICTIONARIES=true") + + conditional(USE_THREADING, "USE_THREADING=true") + + conditional(COMMENT != null && !COMMENT.isEmpty, "COMMENT=" + COMMENT) } \ No newline at end of file diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/PicardBamFunction.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/PicardBamFunction.scala index 427c09f82..76856dc36 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/PicardBamFunction.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/PicardBamFunction.scala @@ -48,14 +48,13 @@ trait PicardBamFunction extends JavaCommandLineFunction { protected def outputBam: File abstract override def commandLine = super.commandLine + - Array( - repeat(" INPUT=", inputBams), - " TMP_DIR=" + jobTempDir, - optional(" OUTPUT=", outputBam), - optional(" COMPRESSION_LEVEL=", compressionLevel), - optional(" VALIDATION_STRINGENCY=", validationStringency), - optional(" SO=", sortOrder), - optional(" MAX_RECORDS_IN_RAM=", maxRecordsInRam), - optional(" ASSUME_SORTED=", assumeSorted), - optional(" CREATE_INDEX=", createIndex)).mkString + repeat("INPUT=", inputBams, spaceSeparated=false) + + required("TMP_DIR=" + jobTempDir) + + optional("OUTPUT=", outputBam, spaceSeparated=false) + + optional("COMPRESSION_LEVEL=", compressionLevel, spaceSeparated=false) + + optional("VALIDATION_STRINGENCY=", validationStringency, spaceSeparated=false) + + optional("SO=", sortOrder, spaceSeparated=false) + + optional("MAX_RECORDS_IN_RAM=", maxRecordsInRam, spaceSeparated=false) + + optional("ASSUME_SORTED=", assumeSorted, spaceSeparated=false) + + optional("CREATE_INDEX=", createIndex, spaceSeparated=false) } diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ReorderSam.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ReorderSam.scala index 72489dc87..b1968bee5 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ReorderSam.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ReorderSam.scala @@ -42,7 +42,7 @@ class ReorderSam extends org.broadinstitute.sting.queue.function.JavaCommandLine this.createIndex = Some(true) this.sortOrder = null override def commandLine = super.commandLine + - " REFERENCE=" + sortReference + - optional(" ALLOW_INCOMPLETE_DICT_CONCORDANCE=", ALLOW_INCOMPLETE_DICT_CONCORDANCE) - optional(" ALLOW_CONTIG_LENGTH_DISCORDANCE=", ALLOW_CONTIG_LENGTH_DISCORDANCE) + required("REFERENCE=" + sortReference) + + optional("ALLOW_INCOMPLETE_DICT_CONCORDANCE=", ALLOW_INCOMPLETE_DICT_CONCORDANCE, spaceSeparated=false) + optional("ALLOW_CONTIG_LENGTH_DISCORDANCE=", ALLOW_CONTIG_LENGTH_DISCORDANCE, spaceSeparated=false) } \ No newline at end of file diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/RevertSam.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/RevertSam.scala index 746ce609e..60d8bfaf8 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/RevertSam.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/RevertSam.scala @@ -52,10 +52,10 @@ class RevertSam extends org.broadinstitute.sting.queue.function.JavaCommandLineF override def outputBam = output this.createIndex = Some(true) override def commandLine = super.commandLine + - conditionalParameter(!restoreOriginalQualities, " RESTORE_ORIGINAL_QUALITIES=false") + - conditionalParameter(!removeDuplicateInformation, " REMOVE_DUPLICATE_INFORMATION=false") + - conditionalParameter(!removeAlignmentInformation, " REMOVE_ALIGNMENT_INFORMATION=false") + - conditionalParameter(!attributesToClear.isEmpty, repeat(" ATTRIBUTE_TO_CLEAR=", attributesToClear)) + - conditionalParameter(sampleAlias != null, " SAMPLE_ALIAS=" + sampleAlias) + - conditionalParameter(libraryName != null, " LIBRARY_NAME=" + libraryName) + conditional(!restoreOriginalQualities, "RESTORE_ORIGINAL_QUALITIES=false") + + conditional(!removeDuplicateInformation, "REMOVE_DUPLICATE_INFORMATION=false") + + conditional(!removeAlignmentInformation, "REMOVE_ALIGNMENT_INFORMATION=false") + + repeat("ATTRIBUTE_TO_CLEAR=", attributesToClear, spaceSeparated=false) + // repeat() returns "" for null/empty list + conditional(sampleAlias != null, "SAMPLE_ALIAS=" + sampleAlias) + + conditional(libraryName != null, "LIBRARY_NAME=" + libraryName) } \ No newline at end of file diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/SamToFastq.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/SamToFastq.scala index 3a4217e60..3eb4e8e06 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/SamToFastq.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/SamToFastq.scala @@ -61,17 +61,17 @@ class SamToFastq extends org.broadinstitute.sting.queue.function.JavaCommandLine this.sortOrder = null override def commandLine = super.commandLine + - " FASTQ=" + fastq + - optional(" SECOND_END_FASTQ=", secondEndFastQ) + - conditionalParameter(outputPerReadGroup, optional(" OUTPUT_PER_RG=", outputPerReadGroup)) + - optional(" OUTPUT_DIR=", outputDir) + - conditionalParameter(!reReverse, optional(" RE_REVERSE=", reReverse)) + - conditionalParameter(includeNonPFReads, optional(" INCLUDE_NON_PF_READS=", includeNonPFReads)) + - optional(" CLIPPING_ATTRIBUTE=", clippingAttribute) + - optional(" CLIPPING_ACTION=", clippingAction) + - conditionalParameter (readOneTrim >= 0, optional(" READ1_TRIM=", readOneTrim)) + - conditionalParameter (readOneMaxBasesToWrite >= 0, optional(" READ1_MAX_BASES_TO_WRITE=", readOneMaxBasesToWrite)) + - conditionalParameter (readTwoTrim >= 0, optional(" READ2_TRIM=", readTwoTrim)) + - conditionalParameter (readTwoMaxBasesToWrite >=0, optional(" READ2_MAX_BASES_TO_WRITE=", readTwoMaxBasesToWrite)) + - conditionalParameter (includeNonPrimaryAlignments, optional(" INCLUDE_NON_PRIMARY_ALIGNMENTS=", includeNonPrimaryAlignments)) + required("FASTQ=" + fastq) + + optional("SECOND_END_FASTQ=", secondEndFastQ, spaceSeparated=false) + + conditional(outputPerReadGroup, "OUTPUT_PER_RG=" + outputPerReadGroup) + + optional("OUTPUT_DIR=", outputDir, spaceSeparated=false) + + conditional(!reReverse, "RE_REVERSE=" + reReverse) + + conditional(includeNonPFReads, "INCLUDE_NON_PF_READS=" + includeNonPFReads) + + optional("CLIPPING_ATTRIBUTE=", clippingAttribute, spaceSeparated=false) + + optional("CLIPPING_ACTION=", clippingAction, spaceSeparated=false) + + conditional(readOneTrim >= 0, "READ1_TRIM=" + readOneTrim) + + conditional(readOneMaxBasesToWrite >= 0, "READ1_MAX_BASES_TO_WRITE=" + readOneMaxBasesToWrite) + + conditional(readTwoTrim >= 0, "READ2_TRIM=" + readTwoTrim) + + conditional(readTwoMaxBasesToWrite >= 0, "READ2_MAX_BASES_TO_WRITE=" + readTwoMaxBasesToWrite) + + conditional(includeNonPrimaryAlignments, "INCLUDE_NON_PRIMARY_ALIGNMENTS=" + includeNonPrimaryAlignments) } \ No newline at end of file diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ValidateSamFile.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ValidateSamFile.scala index 2c8fbc6d9..030e4b07d 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ValidateSamFile.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/picard/ValidateSamFile.scala @@ -50,11 +50,11 @@ class ValidateSamFile extends org.broadinstitute.sting.queue.function.JavaComman override def inputBams = input override def outputBam = output override def commandLine = super.commandLine + - " MODE=" + MODE + - " MAX_OUTPUT=" + MAX_OUTPUT + - " MAX_OPEN_TEMP_FILES=" + MAX_OPEN_TEMP_FILES + - conditionalParameter(!VALIDATE_INDEX, " VALIDATE_INDEX=false") + - conditionalParameter(IGNORE_WARNINGS, " IGNORE_WARNINGS=true") + - conditionalParameter(IS_BISULFITE_SEQUENCED, " IS_BISULFITE_SEQUENCED=true") + - conditionalParameter(IGNORE != null && !IGNORE.isEmpty, repeat(" IGNORE=", IGNORE)) + required("MODE=" + MODE) + + required("MAX_OUTPUT=" + MAX_OUTPUT) + + required("MAX_OPEN_TEMP_FILES=" + MAX_OPEN_TEMP_FILES) + + conditional(!VALIDATE_INDEX, "VALIDATE_INDEX=false") + + conditional(IGNORE_WARNINGS, "IGNORE_WARNINGS=true") + + conditional(IS_BISULFITE_SEQUENCED, "IS_BISULFITE_SEQUENCED=true") + + repeat("IGNORE=", IGNORE, spaceSeparated=false) // repeat() returns "" for null/empty list } \ No newline at end of file diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsIndexFunction.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsIndexFunction.scala index 801a152ec..83a03b904 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsIndexFunction.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsIndexFunction.scala @@ -48,7 +48,10 @@ class SamtoolsIndexFunction extends SamtoolsCommandLineFunction { bamFileIndex = new File(bamFile.getPath + ".bai") } - def commandLine = "%s index %s %s".format(samtools, bamFile, bamFileIndex) + def commandLine = required(samtools) + + required("index") + + required(bamFile) + + required(bamFileIndex) override def dotString = "Index: %s".format(bamFile.getName) } diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsMergeFunction.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsMergeFunction.scala index 2b864def6..aff9a25c0 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsMergeFunction.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/samtools/SamtoolsMergeFunction.scala @@ -55,7 +55,9 @@ class SamtoolsMergeFunction extends SamtoolsCommandLineFunction { )) } - def commandLine = "%s merge%s %s%s".format( - samtools, optional(" -R ", region), - outputBam, repeat(" ", inputBams)) + def commandLine = required(samtools) + + required("merge") + + optional("-R", region) + + required(outputBam) + + repeat(inputBams) } diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/snpeff/SnpEff.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/snpeff/SnpEff.scala index 62f66ec06..259856c17 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/snpeff/SnpEff.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/snpeff/SnpEff.scala @@ -50,11 +50,14 @@ class SnpEff extends JavaCommandLineFunction { @Output(doc="snp eff output") var outVcf: File = _ - override def commandLine = Array( - super.commandLine, - " eff", - if (verbose) " -v" else "", - optional(" -c ", config), - " -i vcf -o vcf %s %s > %s".format(genomeVersion, inVcf, outVcf) - ).mkString + override def commandLine = super.commandLine + + required("eff") + + conditional(verbose, "-v") + + optional("-c", config) + + required("-i", "vcf") + + required("-o", "vcf") + + required(genomeVersion) + + required(inVcf) + + required(">", escape=false) + + required(outVcf) } diff --git a/public/scala/src/org/broadinstitute/sting/queue/function/CommandLineFunction.scala b/public/scala/src/org/broadinstitute/sting/queue/function/CommandLineFunction.scala index ff77503ac..b08832f22 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/function/CommandLineFunction.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/function/CommandLineFunction.scala @@ -110,61 +110,178 @@ trait CommandLineFunction extends QFunction with Logging { } /** - * Repeats parameters with a prefix/suffix if they are set otherwise returns "". - * Skips null, Nil, None. Unwraps Some(x) to x. Everything else is called with x.toString. - * @param prefix Command line prefix per parameter. - * @param params Traversable parameters. - * @param suffix Optional suffix per parameter. - * @param separator Optional separator per parameter. - * @param format Format function if the value has a value - * @return The generated string + * Safely construct a full required command-line argument with consistent quoting, whitespace separation, etc. + * + * @param prefix Prefix to insert before the argument value (eg., "-f") + * @param param The argument value itself + * @param suffix Suffix to append after the argument value + * @param spaceSeparated If true, insert a space between the prefix, param, and suffix + * @param escape If true, quote the generated argument to avoid interpretation by the shell + * @param format Format String used to convert param to a String + * @return The combined and formatted argument, surrounded by whitespace */ - protected def repeat(prefix: String, params: Traversable[_], suffix: String = "", separator: String = "", - format: (String, Any, String) => String = formatValue("%s")) = - if (params == null) - "" - else - params.filter(param => hasValue(param)).map(param => format(prefix, param, suffix)).mkString(separator) + protected def required( prefix: String, param: Any, suffix: String = "", spaceSeparated: Boolean = true, + escape: Boolean = true, format: String = "%s" ): String = { + " %s ".format(formatArgument(prefix, param, suffix, spaceSeparated, escape, format)) + } /** - * Returns parameter with a prefix/suffix if it is set otherwise returns "". - * Does not output null, Nil, None. Unwraps Some(x) to x. Everything else is called with x.toString. - * @param prefix Command line prefix per parameter. - * @param param Parameter to check for a value. - * @param suffix Optional suffix per parameter. - * @param format Format function if the value has a value - * @return The generated string + * Safely construct a one-token required command-line argument with quoting + * + * @param param The command-line argument value + * @return The argument value quoted and surrounded by whitespace */ - protected def optional(prefix: String, param: Any, suffix: String = "", - format: (String, Any, String) => String = formatValue("%s")) = - if (hasValue(param)) format(prefix, param, suffix) else "" + protected def required( param: Any ): String = { + required("", param) + } /** - * Returns "" if the value is null or an empty collection, otherwise return the value.toString. - * @param format Format string if the value has a value - * @param prefix Command line prefix per parameter. - * @param param Parameter to check for a value. - * @param suffix Optional suffix per parameter. - * @return "" if the value is null, or "" if the collection is empty, otherwise the value.toString. + * Safely construct a one-token required command-line argument, and specify whether you want quoting + * + * @param param The command-line argument value + * @param escape If true, quote the generated argument to avoid interpretation by the shell + * @return The argument value, quoted if quoting was requested, and surrounded by whitespace */ - protected def formatValue(format: String)(prefix: String, param: Any, suffix: String): String = - if (CollectionUtils.isNullOrEmpty(param)) - "" - else - prefix + (param match { - case Some(x) => format.format(x) - case x => format.format(x) - }) + suffix + protected def required( param: Any, escape: Boolean ): String = { + required("", param, escape=escape) + } /** - * Returns the parameter if the condition is true. Useful for long string of parameters - * @param condition the condition to validate - * @param param the string to be returned in case condition is true - * @return param if condition is true, "" otherwise + * Safely construct a full optional command-line argument with consistent quoting, whitespace separation, etc. + * If the argument has no value, returns an empty String. + * + * @param prefix Prefix to insert before the argument value (eg., "-f") + * @param param The argument value itself (if null/empty, the method returns empty String) + * @param suffix Suffix to append after the argument value + * @param spaceSeparated If true, insert a space between the prefix, param, and suffix + * @param escape If true, quote the generated argument to avoid interpretation by the shell + * @param format Format String used to convert param to a String + * @return The combined and formatted argument, surrounded by whitespace, or an empty String + * if the argument has no value */ - protected def conditionalParameter(condition: Boolean, param: String): String = - if (condition == true) - param - else + protected def optional( prefix: String, param: Any, suffix: String = "", spaceSeparated: Boolean = true, + escape: Boolean = true, format: String = "%s" ): String = { + if ( hasValue(param) ) " %s ".format(formatArgument(prefix, param, suffix, spaceSeparated, escape, format)) else "" + } + + /** + * Safely construct a one-token optional command-line argument with quoting. + * If the argument has no value, returns an empty String. + * + * @param param The command-line argument value + * @return The argument value quoted and surrounded by whitespace, or an empty String + * if the argument has no value + */ + protected def optional( param: Any ): String = { + optional("", param) + } + + /** + * Safely construct a one-token conditional command-line argument. If the provided condition + * is false, an empty String is returned. + * + * @param condition The condition to check + * @param param The command-line argument value + * @param escape If true, quote the generated argument to avoid interpretation by the shell + * @param format Format String used to convert param to a String + * @return The command-line argument value, quoted if quoting was requested and surrounded + * by whitespace, or an empty String if the argument has no value. + */ + protected def conditional( condition: Boolean, param: Any, escape: Boolean = true, format: String = "%s" ): String = { + if ( condition ) { + " %s ".format(formatArgument("", param, "", false, escape, format)) + } + else { "" + } + } + + /** + * Safely construct a series of full command-line arguments with consistent quoting, whitespace separation, etc. + * + * Each argument value is preceded by a prefix/suffix if they are set. A function can be provided to vary + * each prefix for each argument value (eg., -f:tag1 file1 -f:tag2 file2) -- the default is to use + * the same prefix for all arguments. + * + * @param prefix Prefix to insert before each argument value (eg., "-f") + * @param params The collection of argument values + * @param suffix Suffix to append after each argument value + * @param separator Specifies how to separate the various arguments from each other + * (eg., what should go between '-f' 'file1' and '-f' 'file2'?) + * Default is one space character. + * @param spaceSeparated If true, insert a space between each individual prefix, param, and suffix + * @param escape If true, quote the generated argument to avoid interpretation by the shell + * @param format Format String used to convert each individual param within params to a String + * @param formatPrefix Function mapping (prefix, argumentValue) pairs to prefixes. Can be used to + * vary each prefix depending on the argument value (useful for tags, etc.). + * Default is to use the same prefix for all argument values. + * @return The series of command-line arguments, quoted and whitespace-delimited as requested, + * or an empty String if params was null/Nil/None. + */ + protected def repeat(prefix: String, params: Traversable[_], suffix: String = "", separator: String = " ", + spaceSeparated: Boolean = true, escape: Boolean = true, format: String = "%s", + formatPrefix: (String, Any) => String = (prefix, value) => prefix): String = { + if (CollectionUtils.isNullOrEmpty(params)) + "" + else + " %s ".format(params.filter(param => hasValue(param)).map(param => formatArgument(formatPrefix(prefix, param), param, suffix, spaceSeparated, escape, format)).mkString(separator)) + } + + /** + * Safely construct a series of one-token command-line arguments with quoting and space separation. + * + * @param params The collection of argument values + * @return The argument values quoted and space-delimited, or an empty String if params was null/Nil/None + */ + protected def repeat( params: Traversable[_] ): String = { + repeat("", params) + } + + /** + * Given an (optional) prefix, an argument value, and an (optional) suffix, formats a command-line + * argument with the specified level of quoting and space-separation. + * + * Helper method for required(), optional(), conditional(), and repeat() -- do not use this + * method directly! + * + * @param prefix Prefix to insert before the argument value (eg., "-f"). Ignored if empty/null. + * @param param The argument value itself. If this is Some(x), it is unwrapped to x before processing. + * @param suffix Suffix to append after the argument value. Ignored if empty/null. + * @param spaceSeparated If true, insert a space between the prefix, param, and suffix + * @param escape If true, quote the generated argument to avoid interpretation by the shell + * @param paramFormat Format string used to convert param to a String + * @return The combined and formatted argument, NOT surrounded by any whitespace. + * Returns an empty String if param was null/empty. + */ + protected def formatArgument( prefix: String, param: Any, suffix: String, spaceSeparated: Boolean, escape: Boolean, + paramFormat: String ): String = { + if (CollectionUtils.isNullOrEmpty(param)) { + return "" + } + + // Trim leading and trailing whitespace off our three tokens, and unwrap Some(x) to x for the param + val trimmedValues : List[String] = List((if ( prefix != null ) prefix.trim else ""), + (param match { + case Some(x) => paramFormat.format(x).trim + case x => paramFormat.format(x).trim + }), + (if ( suffix != null ) suffix.trim else "")) + var joinedArgument : String = null + + // If the user requested space-separation, join the tokens with a space, and escape individual + // NON-EMPTY tokens if escaping was requested (eg., ("-f", "foo", "") -> "'-f' 'foo'") + if ( spaceSeparated ) { + joinedArgument = trimmedValues.map(x => if ( x.length > 0 && escape ) ShellUtils.escapeShellArgument(x) else x).mkString(" ").trim() + } + + // Otherwise join the tokens without any intervening whitespace, and if quoting was requested + // quote the entire concatenated value (eg., ("-Xmx", "4", "G") -> "'-Xmx4G'") + else { + joinedArgument = if ( escape ) ShellUtils.escapeShellArgument(trimmedValues.mkString("")) else trimmedValues.mkString("") + } + + // If the user requested escaping and we ended up with an empty String after joining, quote the empty + // String to preserve the command line token. Otherwise just return the joined argument + if ( joinedArgument.length == 0 && escape ) ShellUtils.escapeShellArgument(joinedArgument) else joinedArgument + } } diff --git a/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala b/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala index 4a50a72ac..8cbc2c577 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala @@ -49,18 +49,6 @@ trait JavaCommandLineFunction extends CommandLineFunction { */ var javaMemoryLimit: Option[Double] = None - /** - * Returns the java executable to run. - */ - def javaExecutable: String = { - if (jarFile != null) - "-jar " + jarFile - else if (javaMainClass != null) - "-cp \"%s\" %s".format(javaClasspath.mkString(File.pathSeparator), javaMainClass) - else - null - } - override def freezeFieldValues() { super.freezeFieldValues() @@ -71,11 +59,25 @@ trait JavaCommandLineFunction extends CommandLineFunction { javaClasspath = JavaCommandLineFunction.currentClasspath } - def javaOpts = "%s -Djava.io.tmpdir=%s" - .format(optional(" -Xmx", javaMemoryLimit.map(gb => (gb * 1024).ceil.toInt), "m"), jobTempDir) + /** + * Returns the java executable to run. + */ + def javaExecutable: String = { + if (jarFile != null) + required("-jar", jarFile) + else if (javaMainClass != null) + required("-cp", javaClasspath.mkString(File.pathSeparator)) + + required(javaMainClass) + else + null + } - def commandLine = "java%s %s" - .format(javaOpts, javaExecutable) + def javaOpts = required("-Xmx", javaMemoryLimit.map(gb => (gb * 1024).ceil.toInt), "m", spaceSeparated=false) + + required("-Djava.io.tmpdir=", jobTempDir, spaceSeparated=false) + + def commandLine = required("java") + + javaOpts + + javaExecutable } object JavaCommandLineFunction { diff --git a/public/scala/src/org/broadinstitute/sting/queue/util/ShellUtils.scala b/public/scala/src/org/broadinstitute/sting/queue/util/ShellUtils.scala new file mode 100644 index 000000000..3cb1a705a --- /dev/null +++ b/public/scala/src/org/broadinstitute/sting/queue/util/ShellUtils.scala @@ -0,0 +1,36 @@ +package org.broadinstitute.sting.queue.util + +import java.lang.IllegalArgumentException + +object ShellUtils { + + /** + * Escapes the String it's passed so that it will be interpreted literally when + * parsed by sh/bash. Can correctly escape all characters except \0, \r, and \n + * + * Replaces all instances of ' with '\'', and then surrounds the resulting String + * with single quotes. + * + * Examples: + * ab -> 'ab' + * a'b -> 'a'\''b' + * '' -> ''\'''\''' + * + * Since \' is not supported inside single quotes in the shell (ie., '\'' does not work), + * whenever we encounter a single quote we need to terminate the existing single-quoted + * string, place the \' outside of single quotes, and then start a new single-quoted + * string. As long as we don't insert spaces between the separate strings, the shell will + * concatenate them together into a single argument value for us. + * + * @param str the String to escape + * @return the same String quoted so that it will be interpreted literally when + * parsed by sh/bash + */ + def escapeShellArgument ( str : String ) : String = { + if ( str == null ) { + throw new IllegalArgumentException("escapeShellArgument() was passed a null String") + } + + "'" + str.replaceAll("'", "'\\\\''") + "'" + } +} \ No newline at end of file diff --git a/public/scala/test/org/broadinstitute/sting/queue/function/CommandLineFunctionUnitTest.scala b/public/scala/test/org/broadinstitute/sting/queue/function/CommandLineFunctionUnitTest.scala new file mode 100644 index 000000000..eb50c3a2e --- /dev/null +++ b/public/scala/test/org/broadinstitute/sting/queue/function/CommandLineFunctionUnitTest.scala @@ -0,0 +1,171 @@ +package org.broadinstitute.sting.queue.function + +import org.testng.Assert +import org.testng.annotations.{DataProvider, Test} + +// Since "protected" in Scala is subclass-only and doesn't allow package-level access, we need to +// extend a class if we want to test it +class CommandLineFunctionUnitTest extends CommandLineFunction { + def commandLine = "" + + @DataProvider( name="formatArgumentTestData" ) + def formatArgumentDataProvider = { + Array(Array("", "argvalue", "", true, true, "'argvalue'"), + Array("", "argvalue", "", true, false, "argvalue"), + Array("", "argvalue", "", false, true, "'argvalue'"), + Array("", "argvalue", "", false, false, "argvalue"), + Array("-arg", "argvalue", "", true, true, "'-arg' 'argvalue'"), + Array("-arg", "argvalue", "", true, false, "-arg argvalue"), + Array("ARGNAME=", "ARGVALUE", "", false, true, "'ARGNAME=ARGVALUE'"), + Array("ARGNAME=", "ARGVALUE", "", false, false, "ARGNAME=ARGVALUE"), + Array("-Xmx", "4", "G", true, true, "'-Xmx' '4' 'G'"), + Array("-Xmx", "4", "G", true, false, "-Xmx 4 G"), + Array("-Xmx", "4", "G", false, true, "'-Xmx4G'"), + Array("-Xmx", "4", "G", false, false, "-Xmx4G"), + Array("", "", "", true, true, "''"), + Array("", "", "", true, false, ""), + Array("", "", "", false, true, "''"), + Array("", "", "", false, false, ""), + Array("", null, "", true, true, ""), + Array("", Nil, "", true, true, ""), + Array("", None, "", true, true, ""), + Array(null, null, null, true, true, ""), + Array("", Some("argvalue"), "", true, true, "'argvalue'") + ) + } + + @Test( dataProvider="formatArgumentTestData" ) + def testFormatArgument( prefix: String, param: Any, suffix: String, spaceSeparated: Boolean, escape: Boolean, expectedReturnValue: String ) { + Assert.assertEquals(formatArgument(prefix, param, suffix, spaceSeparated, escape, "%s"), + expectedReturnValue) + } + + @Test + def testFormatArgumentCustomFormatString() { + Assert.assertEquals(formatArgument("", "argvalue", "", true, true, "%.3s"), "'arg'") + } + + @DataProvider( name = "requiredTestData" ) + def requiredDataProvider = { + Array(Array("", "argvalue", "", true, true, " 'argvalue' "), + Array("", "argvalue", "", true, false, " argvalue "), + Array("", "argvalue", "", false, true, " 'argvalue' "), + Array("", "argvalue", "", false, false, " argvalue "), + Array("-arg", "argvalue", "", true, true, " '-arg' 'argvalue' "), + Array("-arg", "argvalue", "", true, false, " -arg argvalue "), + Array("ARGNAME=", "ARGVALUE", "", false, true, " 'ARGNAME=ARGVALUE' "), + Array("ARGNAME=", "ARGVALUE", "", false, false, " ARGNAME=ARGVALUE "), + Array("-Xmx", "4", "G", true, true, " '-Xmx' '4' 'G' "), + Array("-Xmx", "4", "G", true, false, " -Xmx 4 G "), + Array("-Xmx", "4", "G", false, true, " '-Xmx4G' "), + Array("-Xmx", "4", "G", false, false, " -Xmx4G "), + Array("", "", "", true, true, " '' "), + Array("", "", "", true, false, " "), + Array("", "", "", false, true, " '' "), + Array("", "", "", false, false, " "), + Array("", null, "", true, true, " "), + Array("", Nil, "", true, true, " "), + Array("", None, "", true, true, " ") + ) + } + + @Test( dataProvider="requiredTestData" ) + def testRequired( prefix: String, param: Any, suffix: String, spaceSeparated: Boolean, escape: Boolean, expectedReturnValue: String ) { + Assert.assertEquals(required(prefix, param, suffix, spaceSeparated, escape), + expectedReturnValue) + } + + @DataProvider( name = "optionalTestData" ) + def optionalDataProvider = { + Array(Array("-arg", "argvalue", "", true, true, " '-arg' 'argvalue' "), + Array("-arg", null, "", true, true, ""), + Array("-arg", Nil, "", true, true, ""), + Array("-arg", None, "", true, true, ""), + Array("-arg", "", "", true, true, " '-arg' ") + ) + } + + @Test( dataProvider="optionalTestData" ) + def testOptional( prefix: String, param: Any, suffix: String, spaceSeparated: Boolean, escape: Boolean, expectedReturnValue: String ) { + Assert.assertEquals(optional(prefix, param, suffix, spaceSeparated, escape), + expectedReturnValue) + } + + @DataProvider( name = "conditionalTestData" ) + def conditionalDataProvider = { + Array(Array(true, "-FLAG", true, " '-FLAG' "), + Array(true, "-FLAG", false, " -FLAG "), + Array(false, "-FLAG", true, ""), + Array(false, "-FLAG", false, ""), + Array(true, null, true, " "), + Array(true, Nil, true, " "), + Array(true, None, true, " "), + Array(false, null, true, ""), + Array(false, Nil, true, ""), + Array(false, None, true, "") + ) + } + + @Test( dataProvider="conditionalTestData" ) + def testConditional( condition: Boolean, param: Any, escape: Boolean, expectedReturnValue: String ) { + Assert.assertEquals(conditional(condition, param, escape), + expectedReturnValue) + } + + @DataProvider( name = "repeatTestData" ) + def repeatDataProvider = { + Array(Array("", List("a", "bc", "d"), "", " ", true, true, " 'a' 'bc' 'd' "), + Array("", List("a", "bc", "d"), "", " ", true, false, " a bc d "), + Array("", List("a", "bc", "d"), "", "", true, true, " 'a''bc''d' "), + Array("", List("a", "bc", "d"), "", "", true, false, " abcd "), + Array("-f", List("file1", "file2", "file3"), "", " ", true, true, " '-f' 'file1' '-f' 'file2' '-f' 'file3' "), + Array("-f", List("file1", "file2", "file3"), "", " ", true, false, " -f file1 -f file2 -f file3 "), + Array("-f", List("file1", "file2", "file3"), "", " ", false, true, " '-ffile1' '-ffile2' '-ffile3' "), + Array("-f", List("file1", "file2", "file3"), "", " ", false, false, " -ffile1 -ffile2 -ffile3 "), + Array("-f", List("file1", "file2", "file3"), "", "", false, true, " '-ffile1''-ffile2''-ffile3' "), + Array("-f", List("file1", "file2", "file3"), "", "", false, false, " -ffile1-ffile2-ffile3 "), + Array("-f", List("file1", "file2", "file3"), "suffix", " ", true, true, " '-f' 'file1' 'suffix' '-f' 'file2' 'suffix' '-f' 'file3' 'suffix' "), + Array("-f", List("file1", "file2", "file3"), "suffix", " ", true, false, " -f file1 suffix -f file2 suffix -f file3 suffix "), + Array("-f", List("file1", "file2", "file3"), "suffix", " ", false, true, " '-ffile1suffix' '-ffile2suffix' '-ffile3suffix' "), + Array("-f", List("file1", "file2", "file3"), "suffix", " ", false, false, " -ffile1suffix -ffile2suffix -ffile3suffix "), + Array("-f", null, "", " ", true, true, ""), + Array("-f", Nil, "", " ", true, true, "") + ) + } + + @Test( dataProvider="repeatTestData" ) + def testRepeat( prefix: String, params: Traversable[_], suffix: String, separator: String, + spaceSeparated: Boolean, escape: Boolean, expectedReturnValue: String ) { + Assert.assertEquals(repeat(prefix, params, suffix, separator, spaceSeparated, escape), + expectedReturnValue) + } + + // Need to test None separately due to implicit conversion issues when using None in a TestNG data provider + @Test + def testRepeatNone() { + testRepeat("", None, "", " ", true, true, "") + } + + @DataProvider( name = "repeatWithPrefixFormattingTestData" ) + def repeatWithPrefixFormattingDataProvider = { + Array(Array("-f", List("file1", "file2", "file3"), "", " ", true, true, (prefix: String, value: Any) => "%s:tag%s".format(prefix, value), + " '-f:tagfile1' 'file1' '-f:tagfile2' 'file2' '-f:tagfile3' 'file3' "), + Array("-f", List("file1", "file2", "file3"), "", " ", true, false, (prefix: String, value: Any) => "%s:tag%s".format(prefix, value), + " -f:tagfile1 file1 -f:tagfile2 file2 -f:tagfile3 file3 "), + Array("", List("file1", "file2", "file3"), "", " ", true, true, (prefix: String, value: Any) => "-%s".format(value), + " '-file1' 'file1' '-file2' 'file2' '-file3' 'file3' "), + Array("-f", null, "", " ", true, true, (prefix: String, value: Any) => "%s:tag%s".format(prefix, value), + ""), + Array("-f", Nil, "", " ", true, true, (prefix: String, value: Any) => "%s:tag%s".format(prefix, value), + "") + ) + } + + @Test( dataProvider = "repeatWithPrefixFormattingTestData" ) + def testRepeatWithPrefixFormatting( prefix: String, params: Traversable[_], suffix: String, separator: String, + spaceSeparated: Boolean, escape: Boolean, formatPrefix: (String, Any) => String, + expectedReturnValue: String ) { + Assert.assertEquals(repeat(prefix, params, suffix, separator, spaceSeparated, escape, "%s", formatPrefix), + expectedReturnValue) + } +} \ No newline at end of file diff --git a/public/scala/test/org/broadinstitute/sting/queue/util/ShellUtilsUnitTest.scala b/public/scala/test/org/broadinstitute/sting/queue/util/ShellUtilsUnitTest.scala new file mode 100644 index 000000000..76585d207 --- /dev/null +++ b/public/scala/test/org/broadinstitute/sting/queue/util/ShellUtilsUnitTest.scala @@ -0,0 +1,57 @@ +package org.broadinstitute.sting.queue.util + +import org.testng.annotations.Test +import org.testng.Assert +import java.io.{InputStreamReader, BufferedReader} + +class ShellUtilsUnitTest { + + @Test + def testEscapeShellArgumentOneCharSequences() { + // Test all ASCII characters except \0, \n, and \r, which we do not support escaping + for ( asciiCode <- 1 to 127 if asciiCode != 10 && asciiCode != 13 ) { + val originalString: String = "%c".format(asciiCode.toChar) + val quotedString: String = ShellUtils.escapeShellArgument(originalString) + + val child : Process = new ProcessBuilder("/bin/sh", "-c", "printf \"%s\" " + quotedString).start() + val childReader : BufferedReader = new BufferedReader(new InputStreamReader(child.getInputStream)) + val childOutputBuffer : StringBuilder = new StringBuilder + + val childReaderThread : Thread = new Thread(new Runnable() { + def run() { + var line : String = childReader.readLine() + + while ( line != null ) { + childOutputBuffer.append(line) + line = childReader.readLine() + } + } + }) + childReaderThread.start() + + val childReturnValue = child.waitFor() + childReaderThread.join() + + childReader.close() + val childOutput = childOutputBuffer.toString() + + if ( childReturnValue != 0 ) { + Assert.fail("With character ASCII %d, sh child process returned: %d".format(asciiCode, childReturnValue)) + } + else if ( ! originalString.equals(childOutput) ) { + Assert.fail("With character ASCII %d, sh child process output \"%s\" instead of the expected \"%s\"".format( + asciiCode, childOutput, originalString)) + } + } + } + + @Test(expectedExceptions = Array(classOf[IllegalArgumentException])) + def testEscapeShellArgumentNullString() { + ShellUtils.escapeShellArgument(null) + } + + @Test + def testEscapeShellArgumentEmptyString() { + Assert.assertEquals(ShellUtils.escapeShellArgument(""), "''") + } +} \ No newline at end of file From 1ba03a5e7234f6e5a656801aaf31b7a2fa931500 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Mon, 5 Dec 2011 14:06:00 -0500 Subject: [PATCH 2/3] Use optional() instead of required() to construct javaMemoryLimit argument in JavaCommandLineFunction --- .../sting/queue/function/JavaCommandLineFunction.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala b/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala index 8cbc2c577..5b19cf9b6 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/function/JavaCommandLineFunction.scala @@ -72,7 +72,7 @@ trait JavaCommandLineFunction extends CommandLineFunction { null } - def javaOpts = required("-Xmx", javaMemoryLimit.map(gb => (gb * 1024).ceil.toInt), "m", spaceSeparated=false) + + def javaOpts = optional("-Xmx", javaMemoryLimit.map(gb => (gb * 1024).ceil.toInt), "m", spaceSeparated=false) + required("-Djava.io.tmpdir=", jobTempDir, spaceSeparated=false) def commandLine = required("java") + From 677bea0abdb1010da084818a0b8903f4e6ea1dbf Mon Sep 17 00:00:00 2001 From: Khalid Shakir Date: Mon, 5 Dec 2011 23:04:33 -0500 Subject: [PATCH 3/3] Right aligning GATKReport numeric columns and updated MD5s in tests. PreQC parses file with spaces in sample names by using tabs only. PostQC allows passing the file names for the evals so that flanks can be evaled. BaseTest's network temp dir now adds the user name to the path so files aren't created in the root. HybridSelectionPipeline: - Updated to latest versions of reference data. - Refactored Picard parsing code replacing YAML. --- .../sting/gatk/report/GATKReportColumn.java | 44 +++++-- .../report/GATKReportColumnFormat.java} | 50 ++++---- .../sting/gatk/report/GATKReportTable.java | 11 +- .../diffengine/GATKReportDiffableReader.java | 4 +- .../sting/pipeline/Pipeline.java | 62 ---------- .../sting/pipeline/PipelineProject.java | 115 ------------------ .../sting/utils/io/IOUtils.java | 23 ++++ .../utils/yaml/FieldOrderComparator.java | 52 -------- .../utils/yaml/StingYamlRepresenter.java | 88 -------------- .../sting/utils/yaml/YamlUtils.java | 107 ---------------- .../org/broadinstitute/sting/BaseTest.java | 3 +- .../sting/gatk/report/GATKReportUnitTest.java | 28 +++++ .../DiffObjectsIntegrationTest.java | 4 +- .../VariantEvalIntegrationTest.java | 44 +++---- .../VCFStreamingIntegrationTest.java | 2 +- .../sting/pipeline/PipelineUnitTest.java | 88 -------------- 16 files changed, 143 insertions(+), 582 deletions(-) rename public/java/src/org/broadinstitute/sting/{pipeline/PipelineSample.java => gatk/report/GATKReportColumnFormat.java} (55%) delete mode 100644 public/java/src/org/broadinstitute/sting/pipeline/Pipeline.java delete mode 100644 public/java/src/org/broadinstitute/sting/pipeline/PipelineProject.java delete mode 100644 public/java/src/org/broadinstitute/sting/utils/yaml/FieldOrderComparator.java delete mode 100644 public/java/src/org/broadinstitute/sting/utils/yaml/StingYamlRepresenter.java delete mode 100644 public/java/src/org/broadinstitute/sting/utils/yaml/YamlUtils.java delete mode 100644 public/java/test/org/broadinstitute/sting/pipeline/PipelineUnitTest.java diff --git a/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportColumn.java b/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportColumn.java index 6452c7b2b..5a6490afe 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportColumn.java +++ b/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportColumn.java @@ -1,6 +1,8 @@ package org.broadinstitute.sting.gatk.report; -import java.util.TreeMap; +import org.apache.commons.lang.math.NumberUtils; + +import java.util.*; /** * Holds values for a column in a GATK report table @@ -16,12 +18,9 @@ public class GATKReportColumn extends TreeMap { * * @param columnName the name of the column * @param defaultValue the default value of the column - * @param display if true, the column will be displayed in the final output + * @param display if true, the column will be displayed in the final output + * @param format format string */ - public GATKReportColumn(String columnName, Object defaultValue, boolean display) { - this(columnName, defaultValue, display, null); - } - public GATKReportColumn(String columnName, Object defaultValue, boolean display, String format) { this.columnName = columnName; this.defaultValue = defaultValue; @@ -77,22 +76,47 @@ public class GATKReportColumn extends TreeMap { /** * Get the display width for this column. This allows the entire column to be displayed with the appropriate, fixed width. - * @return the width of this column + * @return the format string for this column */ - public int getColumnWidth() { + public GATKReportColumnFormat getColumnFormat() { int maxWidth = columnName.length(); + GATKReportColumnFormat.Alignment alignment = GATKReportColumnFormat.Alignment.RIGHT; for (Object obj : this.values()) { if (obj != null) { - int width = formatValue(obj).length(); + String formatted = formatValue(obj); + int width = formatted.length(); if (width > maxWidth) { maxWidth = width; } + + if (alignment == GATKReportColumnFormat.Alignment.RIGHT) { + if (!isRightAlign(formatted)) { + alignment = GATKReportColumnFormat.Alignment.LEFT; + } + } } } - return maxWidth; + return new GATKReportColumnFormat(maxWidth, alignment); + } + + private static final Collection RIGHT_ALIGN_STRINGS = Arrays.asList( + "null", + "NA", + String.valueOf(Double.POSITIVE_INFINITY), + String.valueOf(Double.NEGATIVE_INFINITY), + String.valueOf(Double.NaN)); + + /** + * Check if the value can be right aligned. Does not trim the values before checking if numeric since it assumes + * the spaces mean that the value is already padded. + * @param value to check + * @return true if the value is a right alignable + */ + protected static boolean isRightAlign(String value) { + return value == null || RIGHT_ALIGN_STRINGS.contains(value) || NumberUtils.isNumber(value); } /** diff --git a/public/java/src/org/broadinstitute/sting/pipeline/PipelineSample.java b/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportColumnFormat.java similarity index 55% rename from public/java/src/org/broadinstitute/sting/pipeline/PipelineSample.java rename to public/java/src/org/broadinstitute/sting/gatk/report/GATKReportColumnFormat.java index 7cd25fed5..6d19a83aa 100644 --- a/public/java/src/org/broadinstitute/sting/pipeline/PipelineSample.java +++ b/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportColumnFormat.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, The Broad Institute + * Copyright (c) 2011, The Broad Institute * * Permission is hereby granted, free of charge, to any person * obtaining a copy of this software and associated documentation @@ -22,41 +22,41 @@ * OTHER DEALINGS IN THE SOFTWARE. */ -package org.broadinstitute.sting.pipeline; - -import java.io.File; -import java.util.Map; -import java.util.TreeMap; +package org.broadinstitute.sting.gatk.report; /** - * Java bean defining a sample for a pipeline. + * Column width and left/right alignment. */ -public class PipelineSample { - private String id; - private Map bamFiles = new TreeMap(); - private Map tags = new TreeMap(); +public class GATKReportColumnFormat { + public static enum Alignment { LEFT, RIGHT } + public int width; + public Alignment alignment; - public String getId() { - return id; + public GATKReportColumnFormat(int width, Alignment alignment) { + this.width = width; + this.alignment = alignment; } - public void setId(String id) { - this.id = id; + public int getWidth() { + return width; } - public Map getBamFiles() { - return bamFiles; + public Alignment getAlignment() { + return alignment; } - public void setBamFiles(Map bamFiles) { - this.bamFiles = bamFiles; + public String getNameFormat() { + return "%-" + width + "s"; } - public Map getTags() { - return tags; - } - - public void setTags(Map tags) { - this.tags = tags; + public String getValueFormat() { + switch (alignment) { + case LEFT: + return "%-" + width + "s"; + case RIGHT: + return "%" + width + "s"; + default: + throw new UnsupportedOperationException("Unknown alignment: " + alignment); + } } } diff --git a/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportTable.java b/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportTable.java index 95c2a14fc..b72b20e0b 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportTable.java +++ b/public/java/src/org/broadinstitute/sting/gatk/report/GATKReportTable.java @@ -608,12 +608,9 @@ public class GATKReportTable { */ public void write(PrintStream out) { // Get the column widths for everything - HashMap columnWidths = new HashMap(); + HashMap columnFormats = new HashMap(); for (String columnName : columns.keySet()) { - int width = columns.get(columnName).getColumnWidth(); - String format = "%-" + String.valueOf(width) + "s"; - - columnWidths.put(columnName, format); + columnFormats.put(columnName, columns.get(columnName).getColumnFormat()); } String primaryKeyFormat = "%-" + getPrimaryKeyColumnWidth() + "s"; @@ -630,7 +627,7 @@ public class GATKReportTable { for (String columnName : columns.keySet()) { if (columns.get(columnName).isDisplayable()) { if (needsPadding) { out.printf(" "); } - out.printf(columnWidths.get(columnName), columnName); + out.printf(columnFormats.get(columnName).getNameFormat(), columnName); needsPadding = true; } @@ -650,7 +647,7 @@ public class GATKReportTable { if (columns.get(columnName).isDisplayable()) { if (needsPadding) { out.printf(" "); } String value = columns.get(columnName).getStringValue(primaryKey); - out.printf(columnWidths.get(columnName), value); + out.printf(columnFormats.get(columnName).getValueFormat(), value); needsPadding = true; } diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/diffengine/GATKReportDiffableReader.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/diffengine/GATKReportDiffableReader.java index ef47ee33c..41b17cc7b 100644 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/diffengine/GATKReportDiffableReader.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/diffengine/GATKReportDiffableReader.java @@ -31,7 +31,6 @@ import org.broadinstitute.sting.gatk.report.GATKReportTable; import java.io.File; import java.io.FileReader; import java.io.IOException; -import java.util.Map; /** @@ -68,7 +67,8 @@ public class GATKReportDiffableReader implements DiffableReader { for ( GATKReportColumn column : table.getColumns().values() ) { DiffNode columnRoot = DiffNode.empty(column.getColumnName(), tableRoot); - columnRoot.add("Width", column.getColumnWidth()); + columnRoot.add("Width", column.getColumnFormat().getWidth()); + // NOTE: as the values are trimmed during parsing left/right alignment is not currently preserved columnRoot.add("Displayable", column.isDisplayable()); int n = 1; diff --git a/public/java/src/org/broadinstitute/sting/pipeline/Pipeline.java b/public/java/src/org/broadinstitute/sting/pipeline/Pipeline.java deleted file mode 100644 index e0e75c353..000000000 --- a/public/java/src/org/broadinstitute/sting/pipeline/Pipeline.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright (c) 2010, 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.pipeline; - -import java.util.ArrayList; -import java.util.List; - -/** - * Java bean for storing a list of samples for a pipeline. - * - * NOTE: This class is used in a very similar way to the classes in - * org.broadinstitute.sting.gatk.datasources.sample. - * - * Both store / load sample information from the file system as YAML. - * - * This package will likely be refactored to share common functionality - * with the other at a future date as requirements coalesce. - * - * - kshakir September 22, 2010 - */ -public class Pipeline { - private PipelineProject project = new PipelineProject(); - private List samples = new ArrayList(); - - public PipelineProject getProject() { - return project; - } - - public void setProject(PipelineProject project) { - this.project = project; - } - - public List getSamples() { - return samples; - } - - public void setSamples(List samples) { - this.samples = samples; - } -} diff --git a/public/java/src/org/broadinstitute/sting/pipeline/PipelineProject.java b/public/java/src/org/broadinstitute/sting/pipeline/PipelineProject.java deleted file mode 100644 index 8d33047bf..000000000 --- a/public/java/src/org/broadinstitute/sting/pipeline/PipelineProject.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright (c) 2010, 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.pipeline; - -import java.io.File; -import java.util.Map; -import java.util.TreeMap; - -/** - * Java bean defining the project for a pipeline. - */ -public class PipelineProject { - private String name; - private File referenceFile; - private File intervalList; - private File genotypeDbsnp; - private File evalDbsnp; - private File refseqTable; - private Map tags = new TreeMap(); - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public File getIntervalList() { - return intervalList; - } - - public void setIntervalList(File intervalList) { - this.intervalList = intervalList; - } - - public File getReferenceFile() { - return referenceFile; - } - - public void setReferenceFile(File referenceFile) { - this.referenceFile = referenceFile; - } - - public File getGenotypeDbsnp() { - return genotypeDbsnp; - } - - public void setGenotypeDbsnp(File genotypeDbsnp) { - this.genotypeDbsnp = genotypeDbsnp; - } - - public String getGenotypeDbsnpType() { - return getDbsnpType(genotypeDbsnp); - } - - public File getEvalDbsnp() { - return evalDbsnp; - } - - public void setEvalDbsnp(File evalDbsnp) { - this.evalDbsnp = evalDbsnp; - } - - public String getEvalDbsnpType() { - return getDbsnpType(evalDbsnp); - } - - public File getRefseqTable() { - return refseqTable; - } - - public void setRefseqTable(File refseqTable) { - this.refseqTable = refseqTable; - } - - public Map getTags() { - return tags; - } - - public void setTags(Map tags) { - this.tags = tags; - } - - private String getDbsnpType(File file) { - if (file == null) - return null; - else if (file.getName().toLowerCase().endsWith(".vcf")) - return "vcf"; - else - return "dbsnp"; - } -} diff --git a/public/java/src/org/broadinstitute/sting/utils/io/IOUtils.java b/public/java/src/org/broadinstitute/sting/utils/io/IOUtils.java index 94c2d4c0b..6f0573b02 100644 --- a/public/java/src/org/broadinstitute/sting/utils/io/IOUtils.java +++ b/public/java/src/org/broadinstitute/sting/utils/io/IOUtils.java @@ -362,4 +362,27 @@ public class IOUtils { org.apache.commons.io.IOUtils.closeQuietly(outputStream); } } + + /** + * Returns a file throwing a UserException if the file cannot be read. + * @param path File path + * @return LineIterator + */ + public static LineIterator lineIterator(String path) { + return lineIterator(new File(path)); + } + + /** + * Returns a file throwing a UserException if the file cannot be read. + * @param file File + * @return LineIterator + */ + public static LineIterator lineIterator(File file) { + try { + return FileUtils.lineIterator(file); + } catch (IOException e) { + throw new UserException.CouldNotReadInputFile(file, e); + } + + } } diff --git a/public/java/src/org/broadinstitute/sting/utils/yaml/FieldOrderComparator.java b/public/java/src/org/broadinstitute/sting/utils/yaml/FieldOrderComparator.java deleted file mode 100644 index 2a043466a..000000000 --- a/public/java/src/org/broadinstitute/sting/utils/yaml/FieldOrderComparator.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright (c) 2010, 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.utils.yaml; - -import org.yaml.snakeyaml.introspector.Property; - -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.List; - -/** - * Orders properties based on the order of the fields in the Java Bean. - */ -class FieldOrderComparator implements Comparator { - private final List propertyOrder; - - public FieldOrderComparator(Class clazz) { - propertyOrder = new ArrayList(); - for (Field field : clazz.getDeclaredFields()) - propertyOrder.add(field.getName()); - } - - @Override - public int compare(Property one, Property two) { - Integer index1 = propertyOrder.indexOf(one.getName()); - Integer index2 = propertyOrder.indexOf(two.getName()); - return index1.compareTo(index2); - } -} diff --git a/public/java/src/org/broadinstitute/sting/utils/yaml/StingYamlRepresenter.java b/public/java/src/org/broadinstitute/sting/utils/yaml/StingYamlRepresenter.java deleted file mode 100644 index 157b1ce27..000000000 --- a/public/java/src/org/broadinstitute/sting/utils/yaml/StingYamlRepresenter.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Copyright (c) 2010, 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.utils.yaml; - -import org.yaml.snakeyaml.introspector.Property; -import org.yaml.snakeyaml.nodes.*; -import org.yaml.snakeyaml.representer.Represent; -import org.yaml.snakeyaml.representer.Representer; - -import java.beans.IntrospectionException; -import java.io.File; -import java.util.Set; -import java.util.TreeSet; - -/** - * A representer with Sting prefered settings. - * - Fields are ordered in the order of the class declaration, instead of alphabetically. - * - Empty maps and sequences are not output. - * - Files are converted to their absolute paths. - */ -public class StingYamlRepresenter extends Representer { - - public StingYamlRepresenter() { - super(); - this.representers.put(File.class, new RepresentFile()); - } - - @Override - protected Set getProperties(Class type) throws IntrospectionException { - TreeSet properties = new TreeSet(new FieldOrderComparator(type)); - properties.addAll(super.getProperties(type)); - return properties; - } - - @Override - protected NodeTuple representJavaBeanProperty(Object javaBean, Property property, - Object propertyValue, Tag customTag) { - NodeTuple tuple = super.representJavaBeanProperty(javaBean, property, propertyValue, customTag); - Node valueNode = tuple.getValueNode(); - if (Tag.NULL.equals(valueNode.getTag())) { - return null;// skip 'null' values - } - if (valueNode instanceof CollectionNode) { - if (Tag.SEQ.equals(valueNode.getTag())) { - SequenceNode seq = (SequenceNode) valueNode; - if (seq.getValue().isEmpty()) { - return null;// skip empty lists - } - } - if (Tag.MAP.equals(valueNode.getTag())) { - MappingNode seq = (MappingNode) valueNode; - if (seq.getValue().isEmpty()) { - return null;// skip empty maps - } - } - } - return tuple; - } - - private class RepresentFile implements Represent { - @Override - public Node representData(Object o) { - return StingYamlRepresenter.this.representScalar(Tag.STR, ((File)o).getPath()); - } - } -} diff --git a/public/java/src/org/broadinstitute/sting/utils/yaml/YamlUtils.java b/public/java/src/org/broadinstitute/sting/utils/yaml/YamlUtils.java deleted file mode 100644 index 715c71efc..000000000 --- a/public/java/src/org/broadinstitute/sting/utils/yaml/YamlUtils.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright (c) 2010, 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.utils.yaml; - -import org.broadinstitute.sting.utils.exceptions.UserException; -import org.yaml.snakeyaml.DumperOptions; -import org.yaml.snakeyaml.Yaml; -import org.yaml.snakeyaml.constructor.Constructor; -import org.yaml.snakeyaml.nodes.Tag; -import org.yaml.snakeyaml.representer.Representer; - -import java.io.File; -import java.io.FileReader; -import java.io.FileWriter; -import java.io.IOException; - -/** - * A collection of utilities for operating on YAML. - * Uses the FLOW style of writing YAML, versus the BLOCK style. - * By default uses a representer that prunes empty lists and maps. - */ -public class YamlUtils { - private static Representer representer = new StingYamlRepresenter(); - private static DumperOptions options = new DumperOptions(); - - static { - options.setCanonical(false); - options.setExplicitRoot(Tag.MAP); - options.setDefaultFlowStyle(DumperOptions.FlowStyle.FLOW); - options.setPrettyFlow(true); - } - - /** - * Serialize an object to the file system. - * @param o Object to serialize. - * @param file Path to write the serialized YAML. - */ - public static void dump(Object o, File file) { - dump(o, file, representer); - } - - /** - * Serialize an object to the file system. - * @param o Object to serialize. - * @param file Path to write the serialized YAML. - * @param representer Custom representer with rules on how to serialize YAML. - */ - public static void dump(Object o, File file, Representer representer) { - Constructor constructor = new Constructor(o.getClass()); - Yaml yaml = new Yaml(constructor, representer, options); - try { - yaml.dump(o, new FileWriter(file)); - } catch (IOException ioe) { - throw new UserException.CouldNotCreateOutputFile(file, ioe); - } - } - - /** - * Deserialize an object from the file system. - * @param clazz Clazz to deserialize. - * @param file Path to read the deserialized YAML. - * @return Object deserialized from the file system. - */ - public static T load(Class clazz, File file) { - return load(clazz, file, representer); - } - - /** - * Deserialize an object from the file system. - * @param clazz Clazz to deserialize. - * @param file Path to read the deserialized YAML. - * @param representer Custom representer with rules on how to deserialize YAML. - * @return Object deserialized from the file system. - */ - @SuppressWarnings("unchecked") - public static T load(Class clazz, File file, Representer representer) { - Constructor constructor = new Constructor(clazz); - Yaml yaml = new Yaml(constructor, representer, options); - try { - return (T) yaml.load(new FileReader(file)); - } catch (IOException ioe) { - throw new UserException.CouldNotReadInputFile(file, ioe); - } - } -} diff --git a/public/java/test/org/broadinstitute/sting/BaseTest.java b/public/java/test/org/broadinstitute/sting/BaseTest.java index f99a105ae..87c03321a 100755 --- a/public/java/test/org/broadinstitute/sting/BaseTest.java +++ b/public/java/test/org/broadinstitute/sting/BaseTest.java @@ -78,7 +78,7 @@ public abstract class BaseTest { public static final String hg19Intervals = intervalsLocation + "whole_exome_agilent_1.1_refseq_plus_3_boosters.Homo_sapiens_assembly19.targets.interval_list"; public static final String hg19Chr20Intervals = intervalsLocation + "whole_exome_agilent_1.1_refseq_plus_3_boosters.Homo_sapiens_assembly19.targets.chr20.interval_list"; - public static final String networkTempDir = "/broad/shptmp/"; + public static final String networkTempDir = "/broad/shptmp/" + System.getProperty("user.name") + "/"; public static final File networkTempDirFile = new File(networkTempDir); public static final File testDirFile = new File("public/testdata/"); @@ -235,6 +235,7 @@ public abstract class BaseTest { */ public static File createNetworkTempFile(String name, String extension) { try { + FileUtils.forceMkdir(networkTempDirFile); File file = File.createTempFile(name, extension, networkTempDirFile); file.deleteOnExit(); return file; diff --git a/public/java/test/org/broadinstitute/sting/gatk/report/GATKReportUnitTest.java b/public/java/test/org/broadinstitute/sting/gatk/report/GATKReportUnitTest.java index f7be1d845..c9b81a9d3 100644 --- a/public/java/test/org/broadinstitute/sting/gatk/report/GATKReportUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/report/GATKReportUnitTest.java @@ -26,6 +26,7 @@ package org.broadinstitute.sting.gatk.report; import org.broadinstitute.sting.BaseTest; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; public class GATKReportUnitTest extends BaseTest { @@ -45,4 +46,31 @@ public class GATKReportUnitTest extends BaseTest { Object validationReportPK = countVariants.getPrimaryKey("none.eval.none.known"); Assert.assertEquals(validationReport.get(validationReportPK, "sensitivity"), "NaN"); } + + @DataProvider(name = "rightAlignValues") + public Object[][] getRightAlignValues() { + return new Object[][] { + new Object[] {null, true}, + new Object[] {"null", true}, + new Object[] {"NA", true}, + new Object[] {"0", true}, + new Object[] {"0.0", true}, + new Object[] {"-0", true}, + new Object[] {"-0.0", true}, + new Object[] {String.valueOf(Long.MAX_VALUE), true}, + new Object[] {String.valueOf(Long.MIN_VALUE), true}, + new Object[] {String.valueOf(Float.MIN_NORMAL), true}, + new Object[] {String.valueOf(Double.MAX_VALUE), true}, + new Object[] {String.valueOf(Double.MIN_VALUE), true}, + new Object[] {String.valueOf(Double.POSITIVE_INFINITY), true}, + new Object[] {String.valueOf(Double.NEGATIVE_INFINITY), true}, + new Object[] {String.valueOf(Double.NaN), true}, + new Object[] {"hello", false} + }; + } + + @Test(dataProvider = "rightAlignValues") + public void testIsRightAlign(String value, boolean expected) { + Assert.assertEquals(GATKReportColumn.isRightAlign(value), expected, "right align of '" + value + "'"); + } } diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/diffengine/DiffObjectsIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/diffengine/DiffObjectsIntegrationTest.java index c8a25c97b..9b79653c6 100644 --- a/public/java/test/org/broadinstitute/sting/gatk/walkers/diffengine/DiffObjectsIntegrationTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/diffengine/DiffObjectsIntegrationTest.java @@ -50,8 +50,8 @@ public class DiffObjectsIntegrationTest extends WalkerTest { @DataProvider(name = "data") public Object[][] createData() { - new TestParams(testDir + "diffTestMaster.vcf", testDir + "diffTestTest.vcf", "ed377322c615abc7dceb97025076078d"); - new TestParams(testDir + "exampleBAM.bam", testDir + "exampleBAM.simple.bam", "02e46f5d2ebb3d49570850595b3f792e"); + new TestParams(testDir + "diffTestMaster.vcf", testDir + "diffTestTest.vcf", "da3dc85a0e35a9aade5520591891b4fa"); + new TestParams(testDir + "exampleBAM.bam", testDir + "exampleBAM.simple.bam", "7dc8200730313e6753237a696296fb73"); return TestParams.getTests(TestParams.class); } diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java index 403ecce78..1555b56d5 100755 --- a/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java @@ -30,7 +30,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("abe943d1aac120d7e75b9b9e5dac2399") + Arrays.asList("f909fd8374f663e983b9b3fda4cf5cf1") ); executeTest("testFunctionClassWithSnpeff", spec); } @@ -50,7 +50,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("5fd9624c7a35ffb79d0feb1e233fc757") + Arrays.asList("081fcaa532c7ba8f23da739389e6f7c3") ); executeTest("testStratifySamplesAndExcludeMonomorphicSites", spec); } @@ -70,7 +70,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("4a8765cd02d36e63f6d0f0c10a6c674b") + Arrays.asList("b3852f84d07c270b8a12874083c3e31b") ); executeTest("testFundamentalsCountVariantsSNPsandIndels", spec); } @@ -91,7 +91,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("4106ab8f742ad1c3138c29220151503c") + Arrays.asList("cf70468b5ebaec408419da69b0a7fcb9") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithNovelty", spec); } @@ -113,7 +113,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("6cee3a8d68307a118944f2df5401ac89") + Arrays.asList("5e3b8b85acfc41365c8208c23abf746b") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithNoveltyAndFilter", spec); } @@ -134,7 +134,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("af5dd27354d5dfd0d2fe03149af09b55") + Arrays.asList("ccdbc50d30ece6d0d3b199c397f03ed3") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithCpG", spec); } @@ -155,7 +155,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("062a231e203671e19aa9c6507710d762") + Arrays.asList("95c690d5af8ed51573eb2f0503dcd9c2") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithFunctionalClass", spec); } @@ -176,7 +176,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("75abdd2b17c0a5e04814b6969a3d4d7e") + Arrays.asList("8e8547eb38b34bec0095b0500fd9641d") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithDegeneracy", spec); } @@ -197,7 +197,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("bdbb5f8230a4a193058750c5e506c733") + Arrays.asList("158a4651a656aea7f84c79548f6fe519") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithSample", spec); } @@ -220,7 +220,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("f076120da22930294840fcc396f5f141") + Arrays.asList("76c8a0b28d2993644120f7afa5833ab2") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithJexlExpression", spec); } @@ -245,7 +245,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("69201f4a2a7a44b38805a4aeeb8830b6") + Arrays.asList("34682193f458b93b39efac00b4fc6723") ); executeTest("testFundamentalsCountVariantsSNPsandIndelsWithMultipleJexlExpressions", spec); } @@ -264,7 +264,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("c3bd3cb6cfb21a8c2b4d5f69104bf6c2") + Arrays.asList("52f6655f1532bcea24b402010d93ce73") ); executeTest("testFundamentalsCountVariantsNoCompRod", spec); } @@ -277,7 +277,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { " --eval " + validationDataLocation + "yri.trio.gatk_glftrio.intersection.annotated.filtered.chr1.vcf" + " --comp:comp_genotypes,VCF3 " + validationDataLocation + "yri.trio.gatk.ug.head.vcf"; WalkerTestSpec spec = new WalkerTestSpec(withSelect(tests, "DP < 50", "DP50") + " " + extraArgs + " -ST CpG -o %s", - 1, Arrays.asList("861f94e3237d62bd5bc00757319241f7")); + 1, Arrays.asList("6fa6e77f149de3d13c31d410a98043a0")); executeTestParallel("testSelect1", spec); } @@ -287,14 +287,14 @@ public class VariantEvalIntegrationTest extends WalkerTest { WalkerTestSpec spec = new WalkerTestSpec(cmdRoot + " -ST CpG --eval:VCF3 " + validationDataLocation + vcfFile + " --comp:VCF3 " + validationDataLocation + "GenotypeConcordanceComp.vcf -noEV -EV GenotypeConcordance -o %s", 1, - Arrays.asList("96f27163f16bb945f19c6623cd6db34e")); + Arrays.asList("9a56c20a7b9a554a7b530f2cb1dd776d")); executeTestParallel("testVEGenotypeConcordance" + vcfFile, spec); } @Test public void testCompVsEvalAC() { String extraArgs = "-T VariantEval -R "+b36KGReference+" -o %s -ST CpG -EV GenotypeConcordance --eval:evalYRI,VCF3 " + validationDataLocation + "yri.trio.gatk.ug.very.few.lines.vcf --comp:compYRI,VCF3 " + validationDataLocation + "yri.trio.gatk.fake.genotypes.ac.test.vcf"; - WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("955c33365e017679047fabec0f14d5e0")); + WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("aeff16bb43be03a2a7e5b9d0108b4999")); executeTestParallel("testCompVsEvalAC",spec); } @@ -312,7 +312,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { @Test public void testCompOverlap() { String extraArgs = "-T VariantEval -R " + b37KGReference + " -L " + validationDataLocation + "VariantEval/pacbio.hg19.intervals --comp:comphapmap " + comparisonDataLocation + "Validated/HapMap/3.3/genotypes_r27_nr.b37_fwd.vcf --eval " + validationDataLocation + "VariantEval/pacbio.ts.recalibrated.vcf -noEV -EV CompOverlap -sn NA12878 -noST -ST Novelty -o %s"; - WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("fb7d989e44bd74c5376cb5732f9f3f64")); + WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("9002023b8aa8fc2c9aac58b8a79bca1e")); executeTestParallel("testCompOverlap",spec); } @@ -324,7 +324,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { " --dbsnp " + b37dbSNP132 + " --eval:evalBI " + validationDataLocation + "VariantEval/ALL.20100201.chr20.bi.sites.vcf" + " -noST -ST Novelty -o %s"; - WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("da5bcb305c5ef207ce175821efdbdefd")); + WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("38ed9d216bd43f1cceceea24146fae38")); executeTestParallel("testEvalTrackWithoutGenotypes",spec); } @@ -336,7 +336,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { " --eval:evalBI " + validationDataLocation + "VariantEval/ALL.20100201.chr20.bi.sites.vcf" + " --eval:evalBC " + validationDataLocation + "VariantEval/ALL.20100201.chr20.bc.sites.vcf" + " -noST -ST Novelty -o %s"; - WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("fde839ece1442388f21a2f0b936756a8")); + WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("453c6b1f7165913e8b1787e22bac1281")); executeTestParallel("testMultipleEvalTracksWithoutGenotypes",spec); } @@ -353,13 +353,13 @@ public class VariantEvalIntegrationTest extends WalkerTest { " -noST -noEV -ST Novelty -EV CompOverlap" + " -o %s"; - WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("1efae6b3b88c752b771e0c8fae24464e")); + WalkerTestSpec spec = new WalkerTestSpec(extraArgs,1,Arrays.asList("61052c19211e7eb61fbbb62db5e40b56")); executeTestParallel("testMultipleCompTracks",spec); } @Test public void testPerSampleAndSubsettedSampleHaveSameResults1() { - String md5 = "bc9bcabc3105e2515d9a2d41506d2de1"; + String md5 = "0edded1cd578db62fa296c99c34a909d"; WalkerTestSpec spec = new WalkerTestSpec( buildCommandLine( @@ -414,7 +414,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("e53546243250634fc03e83b4e61ec55f") + Arrays.asList("ee22604616b3e9fc48a6dcbbf73a056d") ); executeTest("testAlleleCountStrat", spec); } @@ -435,7 +435,7 @@ public class VariantEvalIntegrationTest extends WalkerTest { "-o %s" ), 1, - Arrays.asList("c8086f0525bc13e666afeb670c2e13ae") + Arrays.asList("240369cd651c77e05e8a6659f4a6237e") ); executeTest("testIntervalStrat", spec); } diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/VCFStreamingIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/VCFStreamingIntegrationTest.java index 3a25bc5c1..16b6c97d0 100644 --- a/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/VCFStreamingIntegrationTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/VCFStreamingIntegrationTest.java @@ -98,7 +98,7 @@ public class VCFStreamingIntegrationTest extends WalkerTest { " -EV CompOverlap -noEV -noST" + " -o %s", 1, - Arrays.asList("1f7ed8c0f671dd227ab764624ef0d64c") + Arrays.asList("addf5f4596ddacef40808f6d3d281111") ); executeTest("testVCFStreamingChain", selectTestSpec); diff --git a/public/java/test/org/broadinstitute/sting/pipeline/PipelineUnitTest.java b/public/java/test/org/broadinstitute/sting/pipeline/PipelineUnitTest.java deleted file mode 100644 index 891356670..000000000 --- a/public/java/test/org/broadinstitute/sting/pipeline/PipelineUnitTest.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Copyright (c) 2010, 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.pipeline; - -import org.broadinstitute.sting.pipeline.Pipeline; -import org.broadinstitute.sting.pipeline.PipelineSample; -import org.testng.Assert; -import org.broadinstitute.sting.utils.yaml.YamlUtils; - -import org.testng.annotations.Test; - -import java.io.File; -import java.util.Map; - -public class PipelineUnitTest { - @Test - public void testDumpAndLoad() throws Exception { - Pipeline pipeline = new Pipeline(); - - pipeline.getProject().setName("PRJ_NAME"); - pipeline.getProject().setReferenceFile(new File("my.fasta")); - pipeline.getProject().setGenotypeDbsnp(new File("my.vcf")); - pipeline.getProject().setEvalDbsnp(new File("my.dbsnp")); - pipeline.getProject().getTags().put("testProjectTag", "project value here"); - - PipelineSample sample = new PipelineSample(); - sample.setId("SMP_ID"); - sample.getBamFiles().put("recalibrated", new File("recalibrated.bam")); - sample.getBamFiles().put("cleaned", new File("/absolute/path/to/cleaned.bam")); - sample.getTags().put("testSampleTag", "sample value here"); - - pipeline.getSamples().add(sample); - - File file = File.createTempFile("testDumpAndLoad", ".yaml"); - YamlUtils.dump(pipeline, file); - Pipeline pipelineLoad = YamlUtils.load(Pipeline.class, file); - - Assert.assertEquals(pipelineLoad.getProject().getName(), pipeline.getProject().getName()); - Assert.assertEquals(pipeline.getProject().getReferenceFile(), pipelineLoad.getProject().getReferenceFile()); - Assert.assertEquals(pipeline.getProject().getIntervalList(), pipelineLoad.getProject().getIntervalList()); - Assert.assertEquals(pipeline.getProject().getGenotypeDbsnp(), pipelineLoad.getProject().getGenotypeDbsnp()); - Assert.assertEquals(pipeline.getProject().getGenotypeDbsnpType(), pipelineLoad.getProject().getGenotypeDbsnpType()); - Assert.assertEquals(pipeline.getProject().getEvalDbsnp(), pipelineLoad.getProject().getEvalDbsnp()); - Assert.assertEquals(pipeline.getProject().getEvalDbsnpType(), pipelineLoad.getProject().getEvalDbsnpType()); - - Assert.assertEquals(pipelineLoad.getProject().getTags().size(), pipeline.getProject().getTags().size()); - for (Map.Entry entry : pipeline.getProject().getTags().entrySet()) - Assert.assertEquals(pipeline.getProject().getTags().get(entry.getKey()), entry.getValue()); - - Assert.assertEquals(pipelineLoad.getSamples().size(), pipeline.getSamples().size()); - for (int i = 0; i < pipeline.getSamples().size(); i++) { - PipelineSample pipelineSample = pipeline.getSamples().get(i); - PipelineSample pipelineLoadSample = pipelineLoad.getSamples().get(i); - - Assert.assertEquals(pipelineLoadSample.getId(), pipelineSample.getId()); - - Assert.assertEquals(pipelineLoadSample.getBamFiles().size(), pipelineSample.getBamFiles().size()); - for (Map.Entry entry : pipelineSample.getBamFiles().entrySet()) - Assert.assertEquals(entry.getValue(), pipelineSample.getBamFiles().get(entry.getKey())); - - Assert.assertEquals(pipelineLoadSample.getTags().size(), pipelineSample.getTags().size()); - for (Map.Entry entry : pipelineSample.getTags().entrySet()) - Assert.assertEquals(pipelineSample.getTags().get(entry.getKey()), entry.getValue()); - } - } -}