Skip to content

Commit

Permalink
Womtool flag to list dependencies (broadinstitute#5098)
Browse files Browse the repository at this point in the history
  • Loading branch information
salonishah11 authored and TimurKustov committed Sep 2, 2019
1 parent 8feac6d commit a72f1ef
Show file tree
Hide file tree
Showing 47 changed files with 436 additions and 101 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## 45 Release Notes

### List dependencies flag in Womtool Command Line [(#5098)](https://github.com/broadinstitute/cromwell/pull/5098)

Womtool now outputs the list of files referenced in import statements using `-l` flag for `validate` command.
More info [here](https://cromwell.readthedocs.io/en/stable/WOMtool/)

### BCS backend new Features support

#### New docker registry
Expand Down
19 changes: 17 additions & 2 deletions docs/WOMtool.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ Run the JAR file with no arguments to get the usage message:
java -jar /path/to/womtool.jar <action> <parameters>

Actions:
validate <WDL file>
validate [--list-dependencies] <WDL file>

Performs full validation of the WDL file including syntax
and semantic checking
and semantic checking. -l or --list-dependencies is an optional flag to
list files referenced in import statements.

inputs <WDL file>

Expand Down Expand Up @@ -98,6 +99,20 @@ import "ps.wdl" as ps
^
```
##### --list-dependencies or -l flag
For a successful validation, this will output the list of files referenced in import statements in workflows and their subworkflows.
`$ java -jar womtool.jar validate -l myWdl.wdl`
```hocon
Success!
List of Workflow dependencies are:
/path/to/my/import/myImport.wdl
/path/to/another/import/anotherImport.wdl
https://path-to-http-import/httpImport.wdl
```
### `inputs`
Examine a WDL file with one workflow in it, compute all the inputs needed for that workflow and output a JSON template that the user can fill in with values. The keys in this document should remain unchanged. The values tell you what type the parameter is expecting. For example, if the value were `Array[String]`, then it's expecting a JSON array of JSON strings, like this: `["string1", "string2", "string3"]`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import cromwell.languages.util.LanguageFactoryUtil
import cromwell.languages.{LanguageFactory, ValidatedWomNamespace}
import cwl.preprocessor.CwlReference
import cwl.{Cwl, CwlDecoder}
import wom.ResolvedImportRecord
import wom.core.{WorkflowJson, WorkflowOptionsJson, WorkflowSource}
import wom.executable.WomBundle
import wom.expression.IoFunctionSet
Expand Down Expand Up @@ -55,6 +56,7 @@ class CwlV1_0LanguageFactory(override val config: Config) extends LanguageFactor
}

override def getWomBundle(workflowSource: WorkflowSource,
workflowSourceOrigin: Option[ResolvedImportRecord],
workflowOptionsJson: WorkflowOptionsJson,
importResolvers: List[ImportResolver],
languageFactories: List[LanguageFactory],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package cromwell.languages

import com.typesafe.config.Config
import common.Checked
import common.validation.IOChecked.IOChecked
import common.validation.Checked._
import common.validation.IOChecked.IOChecked
import cromwell.core.{WorkflowId, WorkflowOptions, WorkflowSourceFilesCollection}
import cromwell.languages.util.ImportResolver.ImportResolver
import wom.ResolvedImportRecord
import wom.core._
import wom.executable.WomBundle
import wom.expression.IoFunctionSet
Expand Down Expand Up @@ -34,6 +35,7 @@ trait LanguageFactory {
}

def getWomBundle(workflowSource: WorkflowSource,
workflowSourceOrigin: Option[ResolvedImportRecord],
workflowOptionsJson: WorkflowOptionsJson,
importResolvers: List[ImportResolver],
languageFactories: List[LanguageFactory],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import cromwell.core.path.{DefaultPathBuilder, Path}
import java.nio.file.{Path => NioPath}

import cromwell.core.WorkflowId
import wom.ResolvedImportRecord
import wom.core.WorkflowSource

import scala.concurrent.duration._
Expand All @@ -28,7 +29,7 @@ import scala.util.{Failure, Success, Try}
object ImportResolver {

case class ImportResolutionRequest(toResolve: String, currentResolvers: List[ImportResolver])
case class ResolvedImportBundle(source: WorkflowSource, newResolvers: List[ImportResolver])
case class ResolvedImportBundle(source: WorkflowSource, newResolvers: List[ImportResolver], resolvedImportRecord: ResolvedImportRecord)

trait ImportResolver {
def name: String
Expand Down Expand Up @@ -96,7 +97,7 @@ object ImportResolver {
absolutePathToFile <- makeAbsolute(resolvedPath)
fileContents <- fetchContentFromAbsolutePath(absolutePathToFile)
updatedResolvers = updatedResolverSet(directory, resolvedPath.parent, currentResolvers)
} yield ResolvedImportBundle(fileContents, updatedResolvers)
} yield ResolvedImportBundle(fileContents, updatedResolvers, ResolvedImportRecord(absolutePathToFile.toString))

errorOr.toEither
}
Expand Down Expand Up @@ -183,7 +184,7 @@ object ImportResolver {
val result: Checked[String] = Await.result(responseIO.unsafeToFuture, 15.seconds).body.leftMap(NonEmptyList(_, List.empty))

result map {
ResolvedImportBundle(_, newResolverList(toLookup))
ResolvedImportBundle(_, newResolverList(toLookup), ResolvedImportRecord(toLookup))
}
} match {
case Success(result) => result
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file is used for testing in ImportResolverSpec
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import org.scalatest.{FlatSpec, Matchers}
class ImportResolverSpec extends FlatSpec with Matchers {
behavior of "HttpResolver"

val relativeToGithubRoot = "https://raw.githubusercontent.com/broadinstitute/cromwell/develop/"
val resolvedHttpPath = relativeToGithubRoot + "centaur/src/main/resources/standardTestCases/hello/hello.wdl"

val canon = Map(
"http://abc.com:8000/blah?x=5&y=10" -> "http://abc.com:8000/blah?x=5&y=10",
"http://abc.com:8000/bob/loblaw/law/blog/../../blah/blah" -> "http://abc.com:8000/bob/loblaw/blah/blah",
Expand Down Expand Up @@ -39,62 +42,159 @@ class ImportResolverSpec extends FlatSpec with Matchers {
toResolve shouldBeValid "http://abc.com:8000/blah1/blah2.wdl"
}

it should "resolve a path and store the import in ResolvedImportRecord" in {
val resolver = HttpResolver()
val importUri = "https://raw.githubusercontent.com/broadinstitute/cromwell/develop/centaur/src/main/resources/standardTestCases/hello/hello.wdl"
val resolvedBundle = resolver.innerResolver(importUri, List(resolver))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe resolvedHttpPath
}
}

behavior of "HttpResolver with a 'relativeTo' value"

val relativeHttpResolver = HttpResolver(relativeTo = Some("http://abc.com:8000/blah1/blah2/"))
val relativeToGithubHttpResolver = HttpResolver(relativeTo = Some(relativeToGithubRoot))

it should "resolve an abolute path from a different initial root" in {
it should "resolve an absolute path from a different initial root" in {
val pathToLookup = relativeHttpResolver.pathToLookup("http://def.org:8080/blah3.wdl")
pathToLookup shouldBeValid "http://def.org:8080/blah3.wdl"
}

it should "resolve an absolute path from a different initial root and store it in ResolvedImportRecord" in {
val importUri = "https://github.com/DataBiosphere/job-manager/blob/master/CHANGELOG.md"
val resolvedBundle = relativeToGithubHttpResolver.innerResolver(importUri, List(relativeToGithubHttpResolver))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe importUri
}
}

it should "resolve a relative path" in {
val pathToLookup = relativeHttpResolver.pathToLookup("tools/cool_tool.wdl")
pathToLookup shouldBeValid "http://abc.com:8000/blah1/blah2/tools/cool_tool.wdl"
}

it should "resolve a relative path and store it in ResolvedImportRecord" in {
val importUri = "centaur/src/main/resources/standardTestCases/hello/hello.wdl"
val resolvedBundle = relativeToGithubHttpResolver.innerResolver(importUri, List(relativeToGithubHttpResolver))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe resolvedHttpPath
}
}

it should "resolve a backtracking relative path" in {
val pathToLookup = relativeHttpResolver.pathToLookup("../tools/cool_tool.wdl")
pathToLookup shouldBeValid "http://abc.com:8000/blah1/tools/cool_tool.wdl"
}

it should "resolve a backtracking relative path and store it in ResolvedImportRecord" in {
val importUri = "../develop/centaur/src/main/resources/standardTestCases/hello/hello.wdl"
val resolvedBundle = relativeToGithubHttpResolver.innerResolver(importUri, List(relativeToGithubHttpResolver))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe resolvedHttpPath
}
}

it should "resolve from '/'" in {
val pathToLookup = relativeHttpResolver.pathToLookup("/tools/cool_tool.wdl")
pathToLookup shouldBeValid "http://abc.com:8000/tools/cool_tool.wdl"
}

it should "resolve from '/' and store it in ResolvedImportRecord" in {
val importUri = "/broadinstitute/cromwell/develop/centaur/src/main/resources/standardTestCases/hello/hello.wdl"
val resolvedBundle = relativeToGithubHttpResolver.innerResolver(importUri, List(relativeToGithubHttpResolver))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe resolvedHttpPath
}
}

behavior of "directory resolver from root"

val workingDirectory = sys.props("user.dir")
val rootDirectoryResolver = DirectoryResolver(DefaultPath(Paths.get("/")), customName = None)

it should "resolve a random path" in {
val pathToLookup = rootDirectoryResolver.resolveAndMakeAbsolute("/path/to/file.wdl")
pathToLookup shouldBeValid Paths.get("/path/to/file.wdl")
}

it should "resolve sampleWorkflow path and store absolute path in ResolvedImportRecord" in {
val path = s"$workingDirectory/languageFactories/language-factory-core/src/test/resources/sampleWorkflow.wdl"
val resolvedBundle = rootDirectoryResolver.innerResolver(path, List(rootDirectoryResolver))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe path
}
}

behavior of "unprotected relative directory resolver"

val relativeDirectoryResolver = DirectoryResolver(DefaultPath(Paths.get("/path/to/imports/")), customName = None)

val relativeDirForSampleWf = s"$workingDirectory/languageFactories/language-factory-core/src/test/"
val relativeDirResolverForSampleWf = DirectoryResolver(DefaultPath(Paths.get(relativeDirForSampleWf)), customName = None)

it should "resolve an absolute path" in {
val pathToLookup = relativeDirectoryResolver.resolveAndMakeAbsolute("/path/to/file.wdl")
pathToLookup shouldBeValid Paths.get("/path/to/file.wdl")
}

it should "resolve an absolute path of sampleWorkflow" in {
val path = relativeDirForSampleWf + "resources/sampleWorkflow.wdl"
val resolvedBundle = relativeDirResolverForSampleWf.innerResolver(path, List(relativeDirResolverForSampleWf))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe path
}
}

it should "resolve a relative path" in {
val pathToLookup = relativeDirectoryResolver.resolveAndMakeAbsolute("path/to/file.wdl")
pathToLookup shouldBeValid Paths.get("/path/to/imports/path/to/file.wdl")
}

it should "resolve a relative path for SampleWorkflow" in {
val path = "resources/sampleWorkflow.wdl"
val resolvedBundle = relativeDirResolverForSampleWf.innerResolver(path, List(relativeDirResolverForSampleWf))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe(relativeDirForSampleWf + path)
}
}

behavior of "protected relative directory resolver"

val protectedRelativeDirectoryResolver = DirectoryResolver(DefaultPath(Paths.get("/path/to/imports/")), Some("/path/to/imports/"), customName = None)
val protectedRelativeDirResolverForSampleWf = DirectoryResolver(DefaultPath(Paths.get(relativeDirForSampleWf)), Some(relativeDirForSampleWf), customName = None)

it should "resolve a good relative path" in {
val pathToLookup = protectedRelativeDirectoryResolver.resolveAndMakeAbsolute("path/to/file.wdl")
pathToLookup shouldBeValid Paths.get("/path/to/imports/path/to/file.wdl")
}

it should "resolve a good relative path to sampleWorkflow" in {
val path = "resources/sampleWorkflow.wdl"
val resolvedBundle = protectedRelativeDirResolverForSampleWf.innerResolver(path, List(protectedRelativeDirResolverForSampleWf))

resolvedBundle.map(_.resolvedImportRecord) match {
case Left(e) => fail(s"Expected ResolvedImportBundle but got $e")
case Right(resolvedImport) => resolvedImport.importPath shouldBe(relativeDirForSampleWf + path)
}
}

it should "not resolve an absolute path" in {
val pathToLookup = protectedRelativeDirectoryResolver.resolveAndMakeAbsolute("/path/to/file.wdl")
pathToLookup shouldBeInvalid "/path/to/file.wdl is not an allowed import path"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import wdl.transforms.base.wdlom2wom._
import wdl.transforms.biscayne.ast2wdlom._
import wdl.transforms.biscayne.parsing._
import wdl.transforms.biscayne.wdlom2wom._
import wom.ResolvedImportRecord
import wom.core.{WorkflowJson, WorkflowOptionsJson, WorkflowSource}
import wom.executable.WomBundle
import wom.expression.IoFunctionSet
Expand All @@ -38,7 +39,7 @@ class WdlBiscayneLanguageFactory(override val config: Config) extends LanguageFa

val checked: Checked[ValidatedWomNamespace] = for {
_ <- enabledCheck
bundle <- getWomBundle(workflowSource, source.workflowOptions.asPrettyJson, importResolvers, factories)
bundle <- getWomBundle(workflowSource, workflowSourceOrigin = None, source.workflowOptions.asPrettyJson, importResolvers, factories)
executable <- createExecutable(bundle, source.inputsJson, ioFunctions)
} yield executable

Expand All @@ -47,13 +48,15 @@ class WdlBiscayneLanguageFactory(override val config: Config) extends LanguageFa
}

override def getWomBundle(workflowSource: WorkflowSource,
workflowSourceOrigin: Option[ResolvedImportRecord],
workflowOptionsJson: WorkflowOptionsJson,
importResolvers: List[ImportResolver],
languageFactories: List[LanguageFactory],
convertNestedScatterToSubworkflow : Boolean = true): Checked[WomBundle] = {
val checkEnabled: CheckedAtoB[FileStringParserInput, FileStringParserInput] = CheckedAtoB.fromCheck(x => enabledCheck map(_ => x))
val converter: CheckedAtoB[FileStringParserInput, WomBundle] = checkEnabled andThen stringToAst andThen wrapAst andThen astToFileElement.map(FileElementToWomBundleInputs(_, workflowOptionsJson, convertNestedScatterToSubworkflow, importResolvers, languageFactories, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle
converter.run(FileStringParserInput(workflowSource, "input.wdl"))
.map(b => b.copyResolvedImportRecord(b, workflowSourceOrigin))
}

override def createExecutable(womBundle: WomBundle, inputsJson: WorkflowJson, ioFunctions: IoFunctionSet): Checked[ValidatedWomNamespace] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ import cromwell.languages.{LanguageFactory, ValidatedWomNamespace}
import languages.wdl.draft2.WdlDraft2LanguageFactory._
import mouse.all._
import net.ceedubs.ficus.Ficus._
import wdl.draft2.Draft2ResolvedImportBundle
import wdl.draft2.model.{Draft2ImportResolver, WdlNamespace, WdlNamespaceWithWorkflow}
import wdl.shared.transforms.wdlom2wom.WdlSharedInputParsing
import wdl.transforms.draft2.wdlom2wom.WdlDraft2WomBundleMakers._
import wdl.transforms.draft2.wdlom2wom.WdlDraft2WomExecutableMakers._
import wom.ResolvedImportRecord
import wom.core.{WorkflowJson, WorkflowOptionsJson, WorkflowSource}
import wom.executable.WomBundle
import wom.expression.IoFunctionSet
Expand Down Expand Up @@ -113,7 +115,9 @@ class WdlDraft2LanguageFactory(override val config: Config) extends LanguageFact
}
}


override def getWomBundle(workflowSource: WorkflowSource,
workflowSourceOrigin: Option[ResolvedImportRecord],
workflowOptionsJson: WorkflowOptionsJson,
importResolvers: List[ImportResolver],
languageFactories: List[LanguageFactory],
Expand All @@ -122,7 +126,7 @@ class WdlDraft2LanguageFactory(override val config: Config) extends LanguageFact
_ <- enabledCheck
namespace <- WdlNamespace.loadUsingSource(workflowSource, None, Some(importResolvers map resolverConverter)).toChecked
womBundle <- namespace.toWomBundle
} yield womBundle
} yield womBundle.copyResolvedImportRecord(womBundle, workflowSourceOrigin)
}

override def createExecutable(womBundle: WomBundle, inputs: WorkflowJson, ioFunctions: IoFunctionSet): Checked[ValidatedWomNamespace] = for {
Expand Down Expand Up @@ -154,7 +158,7 @@ class WdlDraft2LanguageFactory(override val config: Config) extends LanguageFact

object WdlDraft2LanguageFactory {
private def resolverConverter(importResolver: ImportResolver): Draft2ImportResolver = str => importResolver.resolver.run(ImportResolutionRequest(str, List.empty)) match {
case Right(imported) => imported.source
case Right(imported) => Draft2ResolvedImportBundle(imported.source, imported.resolvedImportRecord)
case Left(errors) => throw new RuntimeException(s"Bad import $str: ${errors.toList.mkString(System.lineSeparator)}")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class ArrayCoercionsSpec extends FlatSpec with Matchers {

private def createExecutable(workflowSource: WorkflowSource): Unit = {
val result = for {
bundle <- factory.getWomBundle(workflowSource, "{}", List.empty, List.empty)
bundle <- factory.getWomBundle(workflowSource, None, "{}", List.empty, List.empty)
_ <- factory.createExecutable(bundle, inputs = "{}", ioFunctions = new EmptyIoFunctionSet)
} yield ()

Expand Down
Loading

0 comments on commit a72f1ef

Please sign in to comment.