Made 'size' accept empty file? values

backend/src/main/scala/cromwell/backend/wdl/ReadLikeFunctions.scala

@@ -4,7 +4,7 @@ import cromwell.backend.MemorySize
 import cromwell.core.path.PathFactory
 import wdl4s.expression.WdlStandardLibraryFunctions
 import wdl4s.parser.MemoryUnit
-import wdl4s.types.{WdlArrayType, WdlFileType, WdlObjectType, WdlStringType}
+import wdl4s.types._
 import wdl4s.values._
 
 import scala.util.{Failure, Success, Try}
@@ -54,7 +54,7 @@ trait ReadLikeFunctions extends PathFactory { this: WdlStandardLibraryFunctions
       fileSize <- fileSize(fileName)
       _ = if (fileSize > limit) {
         val errorMsg = s"Use of $fileName failed because the file was too big ($fileSize bytes when only files of up to $limit bytes are permissible"
-        throw new FileSizeTooBig(errorMsg)
+        throw FileSizeTooBig(errorMsg)
       }
     } yield ()
 
@@ -119,14 +119,34 @@ trait ReadLikeFunctions extends PathFactory { this: WdlStandardLibraryFunctions
   override def read_boolean(params: Seq[Try[WdlValue]]): Try[WdlBoolean] =
     read_string(params) map { s => WdlBoolean(java.lang.Boolean.parseBoolean(s.value.trim.toLowerCase)) }
 
+  protected def size(file: WdlValue): Try[Double] = Try(buildPath(file.valueString).size.toDouble)
+
   override def size(params: Seq[Try[WdlValue]]): Try[WdlFloat] = {
+    // Inner function: get the memory unit from the second (optional) parameter
     def toUnit(wdlValue: Try[WdlValue]) = wdlValue flatMap { unit => Try(MemoryUnit.fromSuffix(unit.valueString)) }
 
+    // Inner function: is this a file type, or an optional containing a file type?
+    def isOptionalOfFileType(wdlType: WdlType): Boolean = wdlType match {
+      case f if WdlFileType.isCoerceableFrom(f) => true
+      case WdlOptionalType(inner) => isOptionalOfFileType(inner)
+      case _ => false
+    }
+
+    // Inner function: Get the file size, allowing for unpacking of optionals
+    def optionalSafeFileSize(value: WdlValue): Try[Double] = value match {
+      case f if f.isInstanceOf[WdlFile] || WdlFileType.isCoerceableFrom(f.wdlType) => size(f)
+      case WdlOptionalValue(_, Some(o)) => optionalSafeFileSize(o)
+      case WdlOptionalValue(f, None) if isOptionalOfFileType(f) => Success(0d)
+      case _ => Failure(new Exception(s"The 'size' method expects a File argument but instead got ${value.wdlType.toWdlString}."))
+    }
+
+    // Inner function: get the file size and convert into the requested memory unit
     def fileSize(wdlValue: Try[WdlValue], convertTo: Try[MemoryUnit] = Success(MemoryUnit.Bytes)) = {
       for {
         value <- wdlValue
         unit <- convertTo
-      } yield MemorySize(buildPath(value.valueString).size.toDouble, MemoryUnit.Bytes).to(unit).amount
+        fileSize <- optionalSafeFileSize(value)
+      } yield MemorySize(fileSize, MemoryUnit.Bytes).to(unit).amount
     }
 
     params match {

backend/src/test/scala/cromwell/backend/wdl/ReadLikeFunctionsSpec.scala

@@ -0,0 +1,99 @@
+package cromwell.backend.wdl
+
+import cromwell.core.path.PathBuilder
+import org.apache.commons.lang3.NotImplementedException
+import org.scalatest.{FlatSpec, Matchers}
+import wdl4s.expression.PureStandardLibraryFunctionsLike
+import wdl4s.types.{WdlFileType, WdlOptionalType, WdlStringType}
+import wdl4s.values.{WdlFloat, WdlOptionalValue, WdlSingleFile, WdlString, WdlValue}
+
+import scala.util.{Failure, Success, Try}
+
+class ReadLikeFunctionsSpec extends FlatSpec with Matchers {
+
+  behavior of "ReadLikeFunctions.size"
+
+  it should "correctly report a 2048 byte file, in bytes by default" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlSingleFile("blah")))) should be(Success(WdlFloat(2048d)))
+  }
+
+  it should "correctly report a 2048 byte file, in bytes" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlSingleFile("blah")), Success(WdlString("B")))) should be(Success(WdlFloat(2048d)))
+  }
+
+  it should "correctly report a 2048 byte file, in KB" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlSingleFile("blah")), Success(WdlString("KB")))) should be(Success(WdlFloat(2.048d)))
+  }
+
+  it should "correctly report a 2048 byte file, in KiB" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlSingleFile("blah")), Success(WdlString("Ki")))) should be(Success(WdlFloat(2d)))
+  }
+
+  it should "correctly report the size of a supplied, optional, 2048 byte file" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlOptionalValue(WdlFileType, Some(WdlSingleFile("blah")))))) should be(Success(WdlFloat(2048d)))
+  }
+
+  it should "correctly report the size of a supplied, optional optional, 2048 byte file" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlOptionalValue(WdlOptionalType(WdlFileType), Some(WdlOptionalValue(WdlFileType, Some(WdlSingleFile("blah")))))))) should be(Success(WdlFloat(2048d)))
+  }
+
+  it should "correctly report the size of a supplied, optional, 2048 byte file, in MB" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlOptionalValue(WdlFileType, Some(WdlSingleFile("blah")))), Success(WdlString("MB")))) should be(Success(WdlFloat(0.002048d)))
+  }
+
+  it should "correctly report that an unsupplied optional file is empty" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlOptionalValue(WdlFileType, None)))) should be(Success(WdlFloat(0d)))
+  }
+
+  it should "correctly report that an unsupplied File?? is empty" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlOptionalValue(WdlOptionalType(WdlFileType), None)))) should be(Success(WdlFloat(0d)))
+  }
+
+  it should "correctly report that an unsupplied optional file is empty, even in MB" in {
+    val readLike = new TestReadLikeFunctions(Success(2048d))
+    readLike.size(Seq(Success(WdlOptionalValue(WdlFileType, None)), Success(WdlString("MB")))) should be(Success(WdlFloat(0d)))
+  }
+
+  it should "refuse to report file sizes for Strings" in {
+    val readLike = new TestReadLikeFunctions(Failure(new Exception("Bad result: WdlStrings shouldn't even be tried for getting file size")))
+    val oops = readLike.size(Seq(Success(WdlString("blah"))))
+    oops match {
+      case Success(x) => fail(s"Expected a string to not have a file length but instead got $x")
+      case Failure(e) => e.getMessage should be("The 'size' method expects a File argument but instead got String.")
+    }
+  }
+
+  it should "refuse to report file sizes for String?s" in {
+    val readLike = new TestReadLikeFunctions(Failure(new Exception("Bad result: WdlStrings shouldn't even be tried for getting file size")))
+    val oops = readLike.size(Seq(Success(WdlOptionalValue(WdlStringType, None))))
+    oops match {
+      case Success(x) => fail(s"Expected a string to not have a file length but instead got $x")
+      case Failure(e) => e.getMessage should be("The 'size' method expects a File argument but instead got String?.")
+    }
+  }
+
+  it should "pass on underlying size reading errors" in {
+    val readLike = new TestReadLikeFunctions(Failure(new Exception("'size' inner exception, expect me to be passed on")))
+    val oops = readLike.size(Seq(Success(WdlSingleFile("blah"))))
+    oops match {
+      case Success(_) => fail(s"The 'size' engine function didn't return the error generated in the inner 'size' method")
+      case Failure(e) => e.getMessage should be("'size' inner exception, expect me to be passed on")
+    }
+  }
+}
+
+
+class TestReadLikeFunctions(sizeResult: Try[Double]) extends PureStandardLibraryFunctionsLike with ReadLikeFunctions {
+  override protected def size(file: WdlValue): Try[Double] = sizeResult
+  override def pathBuilders: List[PathBuilder] = throw new NotImplementedException("Didn't expect ReadLikefunctionsSpec to need pathBuilders")
+}
+

project/Dependencies.scala

@@ -2,7 +2,7 @@ import sbt._
 
 object Dependencies {
   lazy val lenthallV = "0.25"
-  lazy val wdl4sV = "0.13"
+  lazy val wdl4sV = "0.14-828fa39-SNAP"
 
   lazy val akkaV = "2.4.17"
   lazy val akkaHttpV = "10.0.9"