You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2019/01/16 19:52:48 UTC

[incubator-pinot] branch master updated: Fix the TableConfig toJSONConfig() method (#3694)

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 53923bc  Fix the TableConfig toJSONConfig() method (#3694)
53923bc is described below

commit 53923bce7e0fa1483cc2eb279375065e7e16fc89
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Wed Jan 16 11:52:43 2019 -0800

    Fix the TableConfig toJSONConfig() method (#3694)
    
    toJSONConfig should return nested json object, instead of serialized json string as the key
    Add test to verify that
---
 .../builder/RoutingTableBuilderTestUtil.java       | 18 +++++-----
 .../apache/pinot/common/config/TableConfig.java    | 39 ++++++++++------------
 .../pinot/common/config/TableConfigTest.java       | 14 ++++++--
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/pinot-broker/src/test/java/org/apache/pinot/broker/routing/builder/RoutingTableBuilderTestUtil.java b/pinot-broker/src/test/java/org/apache/pinot/broker/routing/builder/RoutingTableBuilderTestUtil.java
index 1c9ab3e..20113ee 100644
--- a/pinot-broker/src/test/java/org/apache/pinot/broker/routing/builder/RoutingTableBuilderTestUtil.java
+++ b/pinot-broker/src/test/java/org/apache/pinot/broker/routing/builder/RoutingTableBuilderTestUtil.java
@@ -18,29 +18,27 @@
  */
 package org.apache.pinot.broker.routing.builder;
 
-import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 import org.apache.pinot.common.config.RoutingConfig;
 import org.apache.pinot.common.config.TableConfig;
 import org.apache.pinot.common.config.TableNameBuilder;
-import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.CommonConstants.Helix.TableType;
 
 
 /**
  * Util class for routing table builder tests
  */
 public class RoutingTableBuilderTestUtil {
-
   private RoutingTableBuilderTestUtil() {
   }
 
-  public static TableConfig getDynamicComputingTableConfig(String tableName) throws IOException {
-    CommonConstants.Helix.TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableName);
-    TableConfig tableConfig = new TableConfig.Builder(tableType).setTableName(tableName).build();
-    Map<String, String> option = new HashMap<>();
-    option.put(RoutingConfig.ENABLE_DYNAMIC_COMPUTING_KEY, "true");
-    tableConfig.getRoutingConfig().setRoutingTableBuilderOptions(option);
-    return tableConfig;
+  public static TableConfig getDynamicComputingTableConfig(String tableName) {
+    TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableName);
+    RoutingConfig routingConfig = new RoutingConfig();
+    Map<String, String> routingTableBuilderOptions = new HashMap<>();
+    routingTableBuilderOptions.put(RoutingConfig.ENABLE_DYNAMIC_COMPUTING_KEY, "true");
+    routingConfig.setRoutingTableBuilderOptions(routingTableBuilderOptions);
+    return new TableConfig.Builder(tableType).setTableName(tableName).setRoutingConfig(routingConfig).build();
   }
 }
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/TableConfig.java b/pinot-common/src/main/java/org/apache/pinot/common/config/TableConfig.java
index ed99729..444427d 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/config/TableConfig.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/config/TableConfig.java
@@ -41,15 +41,15 @@ import org.apache.pinot.startree.hll.HllConfig;
 @ConfigDoc(value = "Configuration for a table", mandatory = true)
 @ConfigKey("table")
 public class TableConfig {
-  private static final String TABLE_NAME_KEY = "tableName";
-  private static final String TABLE_TYPE_KEY = "tableType";
-  private static final String VALIDATION_CONFIG_KEY = "segmentsConfig";
-  private static final String TENANT_CONFIG_KEY = "tenants";
-  private static final String INDEXING_CONFIG_KEY = "tableIndexConfig";
-  private static final String CUSTOM_CONFIG_KEY = "metadata";
-  private static final String QUOTA_CONFIG_KEY = "quota";
-  private static final String TASK_CONFIG_KEY = "task";
-  private static final String ROUTING_CONFIG_KEY = "routing";
+  public static final String TABLE_NAME_KEY = "tableName";
+  public static final String TABLE_TYPE_KEY = "tableType";
+  public static final String VALIDATION_CONFIG_KEY = "segmentsConfig";
+  public static final String TENANT_CONFIG_KEY = "tenants";
+  public static final String INDEXING_CONFIG_KEY = "tableIndexConfig";
+  public static final String CUSTOM_CONFIG_KEY = "metadata";
+  public static final String QUOTA_CONFIG_KEY = "quota";
+  public static final String TASK_CONFIG_KEY = "task";
+  public static final String ROUTING_CONFIG_KEY = "routing";
 
   @ConfigKey("name")
   @ConfigDoc(value = "The name for the table.", mandatory = true, exampleValue = "myTable")
@@ -158,22 +158,22 @@ public class TableConfig {
   }
 
   @Nonnull
-  public static JsonNode toJSONConfig(@Nonnull TableConfig tableConfig) throws JsonProcessingException {
+  public static JsonNode toJSONConfig(@Nonnull TableConfig tableConfig) {
     ObjectNode jsonConfig = JsonUtils.newObjectNode();
     jsonConfig.put(TABLE_NAME_KEY, tableConfig._tableName);
     jsonConfig.put(TABLE_TYPE_KEY, tableConfig._tableType.toString());
-    jsonConfig.put(VALIDATION_CONFIG_KEY, JsonUtils.objectToString(tableConfig._validationConfig));
-    jsonConfig.put(TENANT_CONFIG_KEY, JsonUtils.objectToString(tableConfig._tenantConfig));
-    jsonConfig.put(INDEXING_CONFIG_KEY, JsonUtils.objectToString(tableConfig._indexingConfig));
-    jsonConfig.put(CUSTOM_CONFIG_KEY, JsonUtils.objectToString(tableConfig._customConfig));
+    jsonConfig.set(VALIDATION_CONFIG_KEY, JsonUtils.objectToJsonNode(tableConfig._validationConfig));
+    jsonConfig.set(TENANT_CONFIG_KEY, JsonUtils.objectToJsonNode(tableConfig._tenantConfig));
+    jsonConfig.set(INDEXING_CONFIG_KEY, JsonUtils.objectToJsonNode(tableConfig._indexingConfig));
+    jsonConfig.set(CUSTOM_CONFIG_KEY, JsonUtils.objectToJsonNode(tableConfig._customConfig));
     if (tableConfig._quotaConfig != null) {
-      jsonConfig.put(QUOTA_CONFIG_KEY, JsonUtils.objectToString(tableConfig._quotaConfig));
+      jsonConfig.set(QUOTA_CONFIG_KEY, JsonUtils.objectToJsonNode(tableConfig._quotaConfig));
     }
     if (tableConfig._taskConfig != null) {
-      jsonConfig.put(TASK_CONFIG_KEY, JsonUtils.objectToString(tableConfig._taskConfig));
+      jsonConfig.set(TASK_CONFIG_KEY, JsonUtils.objectToJsonNode(tableConfig._taskConfig));
     }
     if (tableConfig._routingConfig != null) {
-      jsonConfig.put(ROUTING_CONFIG_KEY, JsonUtils.objectToString(tableConfig._routingConfig));
+      jsonConfig.set(ROUTING_CONFIG_KEY, JsonUtils.objectToJsonNode(tableConfig._routingConfig));
     }
     return jsonConfig;
   }
@@ -311,6 +311,7 @@ public class TableConfig {
     _taskConfig = taskConfig;
   }
 
+  @Nullable
   public RoutingConfig getRoutingConfig() {
     return _routingConfig;
   }
@@ -608,10 +609,6 @@ public class TableConfig {
         _customConfig.setCustomConfigs(new HashMap<>());
       }
 
-      if (_routingConfig == null) {
-        _routingConfig = new RoutingConfig();
-      }
-
       return new TableConfig(_tableName, _tableType, validationConfig, tenantConfig, indexingConfig, _customConfig,
           _quotaConfig, _taskConfig, _routingConfig);
     }
diff --git a/pinot-common/src/test/java/org/apache/pinot/common/config/TableConfigTest.java b/pinot-common/src/test/java/org/apache/pinot/common/config/TableConfigTest.java
index eef3cf1..5a229dd 100644
--- a/pinot-common/src/test/java/org/apache/pinot/common/config/TableConfigTest.java
+++ b/pinot-common/src/test/java/org/apache/pinot/common/config/TableConfigTest.java
@@ -18,6 +18,8 @@
  */
 package org.apache.pinot.common.config;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
@@ -43,8 +45,16 @@ public class TableConfigTest {
       Assert.assertEquals(tableConfig.getIndexingConfig().getLoadMode(), "HEAP");
       Assert.assertNull(tableConfig.getQuotaConfig());
 
-      // Serialize then de-serialize
-      TableConfig tableConfigToCompare = TableConfig.fromJSONConfig(TableConfig.toJSONConfig(tableConfig));
+      // Serialize
+      JsonNode jsonTableConfig = TableConfig.toJSONConfig(tableConfig);
+      // All nested configs should be json objects instead of serialized strings
+      Assert.assertTrue(jsonTableConfig.get(TableConfig.VALIDATION_CONFIG_KEY) instanceof ObjectNode);
+      Assert.assertTrue(jsonTableConfig.get(TableConfig.TENANT_CONFIG_KEY) instanceof ObjectNode);
+      Assert.assertTrue(jsonTableConfig.get(TableConfig.INDEXING_CONFIG_KEY) instanceof ObjectNode);
+      Assert.assertTrue(jsonTableConfig.get(TableConfig.CUSTOM_CONFIG_KEY) instanceof ObjectNode);
+
+      // De-serialize
+      TableConfig tableConfigToCompare = TableConfig.fromJSONConfig(jsonTableConfig);
       Assert.assertEquals(tableConfigToCompare.getTableName(), tableConfig.getTableName());
       Assert.assertNull(tableConfigToCompare.getQuotaConfig());
       Assert.assertNull(tableConfigToCompare.getValidationConfig().getReplicaGroupStrategyConfig());


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org