fix(core): don't try to hdel an empty key set if a stage has data for…

… all its fields
spinnaker · May 12, 2017 · eaa5399 · eaa5399
1 parent 09bc2e6
commit eaa5399
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 2 deletions.
diff --git a/...ovy/com/netflix/spinnaker/orca/pipeline/persistence/jedis/JedisExecutionRepository.groovy b/...ovy/com/netflix/spinnaker/orca/pipeline/persistence/jedis/JedisExecutionRepository.groovy
@@ -590,9 +590,13 @@ class JedisExecutionRepository implements ExecutionRepository {
 
     def serializedStage = serializeStage(stage)
     tx.hmset(key, filterValues(serializedStage, notNull()))
-    tx.hdel(key, serializedStage.keySet().findAll {
+
+    def keysToRemove = serializedStage.keySet().findAll {
       serializedStage[it] == null
-    } as String[])
+    }
+    if (!keysToRemove.empty) {
+      tx.hdel(key, keysToRemove as String[])
+    }
   }
 
   @CompileDynamic

diff --git a/...test/groovy/com/netflix/spinnaker/orca/pipeline/persistence/ExecutionRepositoryTck.groovy b/...test/groovy/com/netflix/spinnaker/orca/pipeline/persistence/ExecutionRepositoryTck.groovy
@@ -22,6 +22,7 @@ import com.netflix.spinnaker.kork.jedis.EmbeddedRedis
 import com.netflix.spinnaker.orca.ExecutionStatus
 import com.netflix.spinnaker.orca.pipeline.model.Orchestration
 import com.netflix.spinnaker.orca.pipeline.model.Pipeline
+import com.netflix.spinnaker.orca.pipeline.model.Stage
 import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository.ExecutionCriteria
 import com.netflix.spinnaker.orca.pipeline.persistence.jedis.JedisExecutionRepository
 import redis.clients.jedis.Jedis
@@ -779,4 +780,28 @@ class JedisExecutionRepositorySpec extends ExecutionRepositoryTck<JedisExecution
       stages.name == ["one-whatever", "one", "two-whatever", "two", "three-whatever", "three"]
     }
   }
+
+  def "can save a stage with all data"() {
+    given:
+    def pipeline = Pipeline
+      .builder()
+      .withApplication("orca")
+      .withName("dummy-pipeline")
+      .withStage("one")
+      .build()
+
+    repository.store(pipeline)
+
+    def stage = newStage(pipeline, "whatever", "one-whatever", [:], pipeline.namedStage("one"), STAGE_BEFORE)
+    stage.lastModified = new Stage.LastModifiedDetails(user: "rfletcher@netflix.com", allowedAccounts: ["whatever"], lastModifiedTime: System.currentTimeMillis())
+    stage.startTime = System.currentTimeMillis()
+    stage.endTime = System.currentTimeMillis()
+    stage.refId = "1<1"
+
+    when:
+    repository.storeStage(stage)
+
+    then:
+    notThrown(Exception)
+  }
 }