From 2e46ca23fd47291a4f783c2ab3d23d8f4bddbb8e Mon Sep 17 00:00:00 2001
From: Miguel Covarrubias <mcovarr@users.noreply.github.com>
Date: Mon, 14 Feb 2022 15:50:34 -0500
Subject: [PATCH] Directory output fixes for PAPI and 'version development' WDL
 [CROM-6875] (#6678)

---
 .../directory_type_output_papi.test           | 12 +++++
 .../directory_type_output.wdl                 | 54 +++++++++++++++++++
 ...inesApiAsyncBackendJobExecutionActor.scala |  2 +-
 ...inesApiAsyncBackendJobExecutionActor.scala |  2 +-
 4 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 centaur/src/main/resources/standardTestCases/directory_type_output_papi.test
 create mode 100644 centaur/src/main/resources/standardTestCases/wdl_biscayne/directory_type_output/directory_type_output.wdl

diff --git a/centaur/src/main/resources/standardTestCases/directory_type_output_papi.test b/centaur/src/main/resources/standardTestCases/directory_type_output_papi.test
new file mode 100644
index 00000000000..f66dfd656b3
--- /dev/null
+++ b/centaur/src/main/resources/standardTestCases/directory_type_output_papi.test
@@ -0,0 +1,12 @@
+name: directory_type_output_papi
+testFormat: workflowsuccess
+tags: ["wdl_biscayne"]
+backends: [Papi]
+
+files {
+  workflow: wdl_biscayne/directory_type_output/directory_type_output.wdl
+}
+
+metadata {
+  workflowName: main
+}
diff --git a/centaur/src/main/resources/standardTestCases/wdl_biscayne/directory_type_output/directory_type_output.wdl b/centaur/src/main/resources/standardTestCases/wdl_biscayne/directory_type_output/directory_type_output.wdl
new file mode 100644
index 00000000000..d3668dd7242
--- /dev/null
+++ b/centaur/src/main/resources/standardTestCases/wdl_biscayne/directory_type_output/directory_type_output.wdl
@@ -0,0 +1,54 @@
+version development
+
+# CROM-6875 repro WDL to exercise Directory outputs. Since the Directory type does not exist in WDL versions 1.0 or
+# draft-2, the bug this is checking for cannot and does not exist in those WDL versions.
+workflow main {
+    call main { input: s1 = "x", s2 = "y" }
+    scatter (f in main.f) {
+        call checker { input: f = f }
+    }
+    output { Array[File] f = main.f }
+}
+
+task main {
+    input {
+        String s1
+        String s2
+    }
+
+    command <<<
+        set -euo pipefail
+        mkdir d
+        touch "d/~{s1}"
+        touch "d/~{s2}"
+        echo -e "d/~{s1}\nd/~{s2}"
+    >>>
+
+    output {
+        Directory d = "d"
+        Array[File] f = read_lines(stdout())
+    }
+
+    runtime {
+        docker: "debian:stable-slim"
+    }
+}
+
+task checker {
+    # Check files were actually created as expected above
+    input {
+        File f
+    }
+
+    command <<<
+        set -euo pipefail
+        [ -f ~{f} ]
+    >>>
+
+    output {
+    }
+
+    runtime {
+        docker: "debian:stable-slim"
+    }
+}
diff --git a/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/PipelinesApiAsyncBackendJobExecutionActor.scala b/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/PipelinesApiAsyncBackendJobExecutionActor.scala
index 319ac966889..a0f899a26ff 100644
--- a/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/PipelinesApiAsyncBackendJobExecutionActor.scala
+++ b/supportedBackends/google/pipelines/v2alpha1/src/main/scala/cromwell/backend/google/pipelines/v2alpha1/PipelinesApiAsyncBackendJobExecutionActor.scala
@@ -292,7 +292,7 @@ class PipelinesApiAsyncBackendJobExecutionActor(standardParams: StandardAsyncExe
       jesOutputs collectFirst {
         case jesOutput if jesOutput.name == makeSafeReferenceName(path) =>
           val pathAsString = jesOutput.cloudPath.pathAsString
-          if (!jesOutput.cloudPath.exists) {
+          if (jesOutput.isFileParameter && !jesOutput.cloudPath.exists) {
             // This is not an error if the path represents a `File?` optional output (the PAPI delocalization script
             // should have failed if this file output was not optional but missing). Throw to produce the correct "empty
             // optional" value for a missing optional file output.
diff --git a/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/PipelinesApiAsyncBackendJobExecutionActor.scala b/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/PipelinesApiAsyncBackendJobExecutionActor.scala
index 1188b427b3d..77370cabc92 100644
--- a/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/PipelinesApiAsyncBackendJobExecutionActor.scala
+++ b/supportedBackends/google/pipelines/v2beta/src/main/scala/cromwell/backend/google/pipelines/v2beta/PipelinesApiAsyncBackendJobExecutionActor.scala
@@ -292,7 +292,7 @@ class PipelinesApiAsyncBackendJobExecutionActor(standardParams: StandardAsyncExe
       jesOutputs collectFirst {
         case jesOutput if jesOutput.name == makeSafeReferenceName(path) =>
           val pathAsString = jesOutput.cloudPath.pathAsString
-          if (!jesOutput.cloudPath.exists) {
+          if (jesOutput.isFileParameter && !jesOutput.cloudPath.exists) {
             // This is not an error if the path represents a `File?` optional output (the PAPI delocalization script
             // should have failed if this file output was not optional but missing). Throw to produce the correct "empty
             // optional" value for a missing optional file output.