Refactor config string processing logic into a util class (#2015)

Refactor config string processing logic into a util class

helix-common/src/main/java/org/apache/helix/constants/InstanceConstants.java

@@ -1,10 +1,11 @@
 package org.apache.helix.constants;
 
 public class InstanceConstants {
+  public static final String INSTANCE_NOT_DISABLED = "INSTANCE_NOT_DISABLED";
+
   public enum InstanceDisabledType {
     CLOUD_EVENT,
     USER_OPERATION,
-    DEFAULT_INSTANCE_DISABLE_TYPE,
-    INSTANCE_NOT_DISABLED
+    DEFAULT_INSTANCE_DISABLE_TYPE
   }
 }

helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java

@@ -29,6 +29,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import org.apache.helix.util.ConfigStringUtil;
 import org.apache.helix.HelixException;
 import org.apache.helix.HelixProperty;
 import org.apache.helix.constants.InstanceConstants;
@@ -66,8 +67,6 @@ public enum InstanceConfigProperty {
   public static final int WEIGHT_NOT_SET = -1;
   public static final int MAX_CONCURRENT_TASK_NOT_SET = -1;
   private static final int TARGET_TASK_THREAD_POOL_SIZE_NOT_SET = -1;
-  private static final String DOMAIN_FIELD_SPLITTER = ",";
-  private static final String DOMAIN_VALUE_JOINER = "=";
 
   private static final Logger _logger = LoggerFactory.getLogger(InstanceConfig.class.getName());
 
@@ -157,21 +156,7 @@ public String getDomainAsString() {
    */
   public Map<String, String> getDomainAsMap() {
     String domain = getDomainAsString();
-    Map<String, String> domainAsMap = new HashMap<>();
-    if (domain == null || domain.isEmpty()) {
-      return domainAsMap;
-    }
-    String[] pathPairs = domain.trim().split(DOMAIN_FIELD_SPLITTER);
-    for (String pair : pathPairs) {
-      String[] values = pair.split(DOMAIN_VALUE_JOINER);
-      if (values.length != 2 || values[0].isEmpty() || values[1].isEmpty()) {
-        throw new IllegalArgumentException(
-            String.format("Domain-Value pair %s is not valid.", pair));
-      }
-      domainAsMap.put(values[0].trim(), values[1].trim());
-    }
-
-    return domainAsMap;
+    return ConfigStringUtil.parseConcatenatedConfig(getDomainAsString());
   }
 
   /**
@@ -187,12 +172,7 @@ public void setDomain(String domain) {
    * @param domainMap domain as a map
    */
   public void setDomain(Map<String, String> domainMap) {
-    String domain = domainMap
-        .entrySet()
-        .stream()
-        .map(entry -> entry.getKey() + DOMAIN_VALUE_JOINER + entry.getValue())
-        .collect(Collectors.joining(DOMAIN_FIELD_SPLITTER));
-    setDomain(domain);
+    setDomain(ConfigStringUtil.concatenateMapping(domainMap));
   }
 
   public int getWeight() {
@@ -287,11 +267,18 @@ public void setInstanceEnabled(boolean enabled) {
     _record.setLongField(InstanceConfigProperty.HELIX_ENABLED_TIMESTAMP.name(),
         System.currentTimeMillis());
     if (enabled) {
-      _record.getSimpleFields().remove(InstanceConfigProperty.HELIX_DISABLED_REASON.toString());
-      _record.getSimpleFields().remove(InstanceConfigProperty.HELIX_DISABLED_TYPE.toString());
+      resetInstanceDisabledTypeAndReason();
     }
   }
 
+  /**
+   * Removes HELIX_DISABLED_REASON and HELIX_DISABLED_TYPE entry from simple field.
+   */
+  public void resetInstanceDisabledTypeAndReason() {
+    _record.getSimpleFields().remove(InstanceConfigProperty.HELIX_DISABLED_REASON.toString());
+    _record.getSimpleFields().remove(InstanceConfigProperty.HELIX_DISABLED_TYPE.toString());
+  }
+
   /**
    * Set the instance disabled reason when instance is disabled.
    * It will be a no-op when instance is enabled.
@@ -327,7 +314,7 @@ public String getInstanceDisabledReason() {
    */
   public String getInstanceDisabledType() {
     if (getInstanceEnabled()) {
-      return InstanceConstants.InstanceDisabledType.INSTANCE_NOT_DISABLED.toString();
+      return InstanceConstants.INSTANCE_NOT_DISABLED;
     }
     return _record.getStringField(InstanceConfigProperty.HELIX_DISABLED_TYPE.toString(),
         InstanceConstants.InstanceDisabledType.DEFAULT_INSTANCE_DISABLE_TYPE.toString());

helix-core/src/main/java/org/apache/helix/util/ConfigStringUtil.java

@@ -0,0 +1,69 @@
+package org.apache.helix.util;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+public final class ConfigStringUtil {
+  private static final String CONCATENATE_CONFIG_SPLITTER = ",";
+  private static final String CONCATENATE_CONFIG_JOINER = "=";
+
+  private ConfigStringUtil() {
+    throw new java.lang.UnsupportedOperationException(
+        "Utility class ConfigStringUtil and cannot be instantiated");
+  }
+
+  /**
+   * Parse a string represented map into a map.
+   * @param inputStr "propName0=propVal0,propName1=propVal1"
+   * @return map {[propName0, propVal0], [propName1, propVal1]}"
+   */
+  public static Map<String, String> parseConcatenatedConfig(String inputStr) {
+    Map<String, String> resultMap = new HashMap<>();
+    if (inputStr == null || inputStr.isEmpty()) {
+      return resultMap;
+    }
+    String[] pathPairs = inputStr.trim().split(CONCATENATE_CONFIG_SPLITTER);
+    for (String pair : pathPairs) {
+      String[] values = pair.split(CONCATENATE_CONFIG_JOINER);
+      if (values.length != 2 || values[0].isEmpty() || values[1].isEmpty()) {
+        throw new IllegalArgumentException(
+            String.format("Domain-Value pair %s is not valid.", pair));
+      }
+      resultMap.put(values[0].trim(), values[1].trim());
+    }
+    return resultMap;
+  }
+
+  /**
+   * Concatenate a map into a string .
+   * @param inputMap {[propName0, propVal0], [propName1, propVal1]}
+   * @return String "propName0=propVal0,propName1=propVal1"
+   */
+  public static String concatenateMapping(Map<String, String> inputMap) {
+    return inputMap
+        .entrySet()
+        .stream()
+        .map(entry -> entry.getKey() + CONCATENATE_CONFIG_JOINER + entry.getValue())
+        .collect(Collectors.joining(CONCATENATE_CONFIG_SPLITTER));
+  }
+}

helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java

@@ -189,7 +189,7 @@ public void testZkHelixAdmin() {
     Assert.assertTrue(
         tool.getInstanceConfig(clusterName, instanceName).getInstanceDisabledReason().isEmpty());
     Assert.assertEquals(tool.getInstanceConfig(clusterName, instanceName).getInstanceDisabledType(),
-        InstanceConstants.InstanceDisabledType.INSTANCE_NOT_DISABLED.toString());
+        InstanceConstants.INSTANCE_NOT_DISABLED);
 
     dummyList.remove("bar");
     dummyList.add("baz");

helix-rest/src/test/java/org/apache/helix/rest/server/TestPerInstanceAccessor.java

@@ -380,7 +380,7 @@ public void updateInstance() throws IOException {
         _configAccessor.getInstanceConfig(CLUSTER_NAME, INSTANCE_NAME).getInstanceEnabled());
     Assert.assertEquals(
         _configAccessor.getInstanceConfig(CLUSTER_NAME, INSTANCE_NAME).getInstanceDisabledType(),
-        InstanceConstants.InstanceDisabledType.INSTANCE_NOT_DISABLED.toString());
+        InstanceConstants.INSTANCE_NOT_DISABLED);
     Assert.assertEquals(
         _configAccessor.getInstanceConfig(CLUSTER_NAME, INSTANCE_NAME).getInstanceDisabledReason(),
         "");