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