From ab4209c82a324a9b3c604cb07045505466c4f272 Mon Sep 17 00:00:00 2001
From: Himanshu <g.himanshu@gmail.com>
Date: Tue, 7 Jun 2016 17:39:41 -0500
Subject: [PATCH] killDataSourceWhitelist in CoordinatorDynamicConfig accepts
 comma separated list of strings in addition to json array of strings so that
 coordinator console can do the updates correctly (#3095)

---
 .../coordinator/CoordinatorDynamicConfig.java | 25 +++++++++++--
 .../http/CoordinatorDynamicConfigTest.java    | 36 +++++++++++++++++--
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/server/src/main/java/io/druid/server/coordinator/CoordinatorDynamicConfig.java b/server/src/main/java/io/druid/server/coordinator/CoordinatorDynamicConfig.java
index f4eaa7a5e2b..82277ac530e 100644
--- a/server/src/main/java/io/druid/server/coordinator/CoordinatorDynamicConfig.java
+++ b/server/src/main/java/io/druid/server/coordinator/CoordinatorDynamicConfig.java
@@ -20,7 +20,10 @@ package io.druid.server.coordinator;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.collect.ImmutableSet;
 
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.Set;
 
 public class CoordinatorDynamicConfig
@@ -47,7 +50,11 @@ public class CoordinatorDynamicConfig
       @JsonProperty("replicationThrottleLimit") int replicationThrottleLimit,
       @JsonProperty("balancerComputeThreads") int balancerComputeThreads,
       @JsonProperty("emitBalancingStats") boolean emitBalancingStats,
-      @JsonProperty("killDataSourceWhitelist") Set<String> killDataSourceWhitelist
+
+      // Type is Object here so that we can support both string and list as
+      // coordinator console can not send array of strings in the update request.
+      // See https://github.com/druid-io/druid/issues/3055
+      @JsonProperty("killDataSourceWhitelist") Object killDataSourceWhitelist
   )
   {
     this.maxSegmentsToMove = maxSegmentsToMove;
@@ -58,7 +65,21 @@ public class CoordinatorDynamicConfig
     this.replicationThrottleLimit = replicationThrottleLimit;
     this.emitBalancingStats = emitBalancingStats;
     this.balancerComputeThreads = Math.max(balancerComputeThreads, 1);
-    this.killDataSourceWhitelist = killDataSourceWhitelist;
+
+    if (killDataSourceWhitelist instanceof String) {
+      String[] tmp = ((String) killDataSourceWhitelist).split(",");
+      this.killDataSourceWhitelist = new HashSet<>();
+      for (int i = 0; i < tmp.length; i++) {
+        String trimmed = tmp[i].trim();
+        if (!trimmed.isEmpty()) {
+          this.killDataSourceWhitelist.add(trimmed);
+        }
+      }
+    } else if (killDataSourceWhitelist instanceof Collection){
+      this.killDataSourceWhitelist = ImmutableSet.copyOf(((Collection) killDataSourceWhitelist));
+    } else {
+      this.killDataSourceWhitelist = ImmutableSet.of();
+    }
   }
 
   @JsonProperty
diff --git a/server/src/test/java/io/druid/server/http/CoordinatorDynamicConfigTest.java b/server/src/test/java/io/druid/server/http/CoordinatorDynamicConfigTest.java
index c4536627e69..a6114a25e89 100644
--- a/server/src/test/java/io/druid/server/http/CoordinatorDynamicConfigTest.java
+++ b/server/src/test/java/io/druid/server/http/CoordinatorDynamicConfigTest.java
@@ -42,7 +42,7 @@ public class CoordinatorDynamicConfigTest
                      + "  \"replicationThrottleLimit\": 1,\n"
                      + "  \"balancerComputeThreads\": 2, \n"
                      + "  \"emitBalancingStats\": true,\n"
-                     + "  \"killDataSourceWhitelist\": [\"test\"]\n"
+                     + "  \"killDataSourceWhitelist\": [\"test1\",\"test2\"]\n"
                      + "}\n";
 
     ObjectMapper mapper = TestHelper.getObjectMapper();
@@ -57,7 +57,39 @@ public class CoordinatorDynamicConfigTest
     );
 
     Assert.assertEquals(
-        new CoordinatorDynamicConfig(1, 1, 1, 1, 1, 1, 2, true, ImmutableSet.of("test")),
+        new CoordinatorDynamicConfig(1, 1, 1, 1, 1, 1, 2, true, ImmutableSet.of("test1", "test2")),
+        actual
+    );
+  }
+
+  @Test
+  public void testSerdeWithStringinKillDataSourceWhitelist() throws Exception
+  {
+    String jsonStr = "{\n"
+                     + "  \"millisToWaitBeforeDeleting\": 1,\n"
+                     + "  \"mergeBytesLimit\": 1,\n"
+                     + "  \"mergeSegmentsLimit\" : 1,\n"
+                     + "  \"maxSegmentsToMove\": 1,\n"
+                     + "  \"replicantLifetime\": 1,\n"
+                     + "  \"replicationThrottleLimit\": 1,\n"
+                     + "  \"balancerComputeThreads\": 2, \n"
+                     + "  \"emitBalancingStats\": true,\n"
+                     + "  \"killDataSourceWhitelist\": \" test1 ,test2 \"\n"
+                     + "}\n";
+
+    ObjectMapper mapper = TestHelper.getObjectMapper();
+    CoordinatorDynamicConfig actual = mapper.readValue(
+        mapper.writeValueAsString(
+            mapper.readValue(
+                jsonStr,
+                CoordinatorDynamicConfig.class
+            )
+        ),
+        CoordinatorDynamicConfig.class
+    );
+
+    Assert.assertEquals(
+        new CoordinatorDynamicConfig(1, 1, 1, 1, 1, 1, 2, true, ImmutableSet.of("test1", "test2")),
         actual
     );
   }