fix(tasks): duplicate empty keys ignored (#3115)

orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/TaskController.groovy

@@ -712,11 +712,19 @@ class TaskController {
     return aStartTime <=> bStartTime ?: b.id <=> a.id
   }
 
+  static boolean shouldReplace(Map.Entry<String, Object> entry, Map variables) {
+    // a duplicate key with an empty value should
+    // not overwrite a previous key with a non empty value
+    return isNullOrEmpty(variables[entry.key]) || !isNullOrEmpty(entry.value)
+  }
+
   private OrchestrationViewModel convert(Execution orchestration) {
     def variables = [:]
     for (stage in orchestration.stages) {
       for (entry in stage.context.entrySet()) {
-        variables[entry.key] = entry.value
+        if (shouldReplace(entry, variables)) {
+          variables[entry.key] = entry.value
+        }
       }
     }
     new OrchestrationViewModel(
@@ -749,6 +757,19 @@ class TaskController {
     return pipelineConfigIds
   }
 
+  private static boolean isNullOrEmpty(Object value) {
+    if (value == null) {
+      return true
+    } else if (value instanceof Collection) {
+      return value.isEmpty()
+    } else if (value instanceof String) {
+      return value.length() == 0
+    } else if (value == [:]) {
+      return true
+    }
+    return false
+  }
+
   @VisibleForTesting
   private static boolean checkObjectMatchesSubset(Object object, Object subset) {
     if (String.isInstance(object) && String.isInstance(subset)) {

orca-web/src/test/groovy/com/netflix/spinnaker/orca/controllers/TaskControllerSpec.groovy

@@ -32,6 +32,7 @@ import org.springframework.mock.web.MockHttpServletResponse
 import org.springframework.test.web.servlet.MockMvc
 import org.springframework.test.web.servlet.setup.MockMvcBuilders
 import spock.lang.Specification
+import spock.lang.Unroll
 
 import java.time.Clock
 import java.time.Instant
@@ -148,6 +149,42 @@ class TaskControllerSpec extends Specification {
     ]
   }
 
+  @Unroll
+  void 'duplicate empty keys are not saved into the list of variables'() {
+    setup:
+    def variables = [
+      mystringkey: "",
+      mystringkey2: "mystringval",
+      mymapkey: [:],
+      mymapkey2: [
+        childkey:"childVal"
+      ],
+      mylistkey: [],
+      mylistkey2: ["why", "oh", "why"]
+    ]
+
+    when:
+    def entry = [(thekey): value].entrySet().first()
+    def result = TaskController.shouldReplace(entry, variables)
+
+    then:
+    result == shouldReplace
+
+    where:
+    thekey            | value    | shouldReplace
+    "mystringkey"     | "hi"     | true
+    "mystringkey2"    | ""       | false
+    "mymapkey"        | [k: "v"] | true
+    "mymapkey2"       | [:]      | false
+    "mylistkey"       | ["hi"]   | true
+    "mylistkey2"      | ["hi"]   | true
+    "mylistkey2"      | []       | false
+    "mynewkey"        | [:]      | true
+    "mynewkey"        | ["hi"]   | true
+    "mynewkey"        | "hi"     | true
+
+  }
+
   void '/tasks returns [] when there are no tasks'() {
     when:
     MockHttpServletResponse response = mockMvc.perform(get('/tasks')).andReturn().response