KYLIN-3559 Use Splitter for splitting String (#364)

* KYLIN-3559 Use Splitter for splitting String * KYLIN-3559 Resolve UT issues * KYLIN-3559 fix IT issues
apache · Nov 28, 2018 · 97ee98b · 97ee98b
1 parent 7bcc698
commit 97ee98b
Show file tree

Hide file tree

Showing 25 changed files with 138 additions and 55 deletions.
diff --git a/core-common/src/main/java/org/apache/kylin/common/util/StringUtil.java b/core-common/src/main/java/org/apache/kylin/common/util/StringUtil.java
@@ -22,6 +22,9 @@
 import java.util.Collection;
 
 import java.util.Locale;
+
+import com.google.common.base.Splitter;
+import com.google.common.collect.Iterables;
 import org.apache.commons.lang.StringUtils;
 
 /**
@@ -159,14 +162,17 @@ public static void appendWithSeparator(StringBuilder src, String append) {
     }
 
     public static String[] splitAndTrim(String str, String splitBy) {
-        String[] split = str.split(splitBy);
-        ArrayList<String> r = new ArrayList<>(split.length);
-        for (String s : split) {
-            s = s.trim();
-            if (!s.isEmpty())
-                r.add(s);
-        }
-        return r.toArray(new String[r.size()]);
+        Splitter splitterWithTrim = Splitter.on(splitBy).trimResults().omitEmptyStrings();
+
+        return Iterables.toArray(splitterWithTrim.split(str), String.class);
+    }
+
+    public static String[] split(String str, String splitBy) {
+        return Iterables.toArray(Splitter.on(splitBy).split(str), String.class);
+    }
+
+    public static String[] splitByComma(String str) {
+        return split(str, ",");
     }
 
     // calculating length in UTF-8 of Java String without actually encoding it

diff --git a/core-common/src/test/java/org/apache/kylin/common/util/StringUtilTest.java b/core-common/src/test/java/org/apache/kylin/common/util/StringUtilTest.java
@@ -0,0 +1,53 @@
+/*
+ * 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.
+ */
+
+package org.apache.kylin.common.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class StringUtilTest {
+    @Test
+    public void splitTest() {
+        String normalText = "Try to make the code better";
+        String[] expected = new String[] { "Try", "to", "make", "the", "code", "better" };
+        Assert.assertArrayEquals(expected, StringUtil.split(normalText, " "));
+
+        // case in http://errorprone.info/bugpattern/StringSplitter
+        expected = new String[] { "" };
+        Assert.assertArrayEquals(expected, StringUtil.split("", ":"));
+
+        expected = new String[] { "", "" };
+        Assert.assertArrayEquals(expected, StringUtil.split(":", ":"));
+
+        expected = new String[] { "1", "2" };
+        Assert.assertArrayEquals(expected, StringUtil.split("1<|>2", "<|>"));
+    }
+
+    @Test
+    public void splitAndTrimTest() {
+        String[] expected = new String[] { "foo", "bar" };
+        Assert.assertArrayEquals(expected, StringUtil.splitAndTrim(" foo... bar. ", "."));
+    }
+
+    @Test
+    public void splitByCommaTest() {
+        String[] expected = new String[] { "Hello", "Kylin" };
+        Assert.assertArrayEquals(expected, StringUtil.splitByComma("Hello,Kylin"));
+    }
+}
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/cli/CubeSignatureRefresher.java b/core-cube/src/main/java/org/apache/kylin/cube/cli/CubeSignatureRefresher.java
@@ -24,6 +24,7 @@
 import org.apache.commons.lang.ArrayUtils;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.persistence.ResourceStore;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.cube.CubeDescManager;
 import org.apache.kylin.cube.CubeManager;
 import org.apache.kylin.cube.model.CubeDesc;
@@ -61,7 +62,7 @@ public void update() {
         if (ArrayUtils.isEmpty(cubeNames)) {
             cubeDescs = cubeDescManager.listAllDesc();
         } else {
-            String[] names = cubeNames[0].split(",");
+            String[] names = StringUtil.splitByComma(cubeNames[0]);
             if (ArrayUtils.isEmpty(names))
                 return;
             cubeDescs = Lists.newArrayListWithCapacity(names.length);

diff --git a/core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DistributedScheduler.java b/core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DistributedScheduler.java
@@ -36,6 +36,7 @@
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.lock.DistributedLock;
 import org.apache.kylin.common.util.SetThreadName;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.job.Scheduler;
 import org.apache.kylin.job.engine.JobEngineConfig;
 import org.apache.kylin.job.exception.ExecuteException;
@@ -150,7 +151,7 @@ public WatcherProcessImpl(String serverName) {
 
         @Override
         public void onUnlock(String path, String nodeData) {
-            String[] paths = path.split("/");
+            String[] paths = StringUtil.split(path, "/");
             String jobId = paths[paths.length - 1];
 
             final Output output = executableManager.getOutput(jobId);

diff --git a/engine-mr/src/main/java/org/apache/kylin/engine/mr/LookupMaterializeContext.java b/engine-mr/src/main/java/org/apache/kylin/engine/mr/LookupMaterializeContext.java
@@ -21,6 +21,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.job.execution.DefaultChainedExecutable;
 
 import com.google.common.collect.Maps;
@@ -74,9 +75,9 @@ public String getAllLookupSnapshotsInString() {
      */
     public static Map<String, String> parseLookupSnapshots(String snapshotsString) {
         Map<String, String> lookupSnapshotMap = Maps.newHashMap();
-        String[] lookupSnapshotEntries = snapshotsString.split(",");
+        String[] lookupSnapshotEntries = StringUtil.splitByComma(snapshotsString);
         for (String lookupSnapshotEntryStr : lookupSnapshotEntries) {
-            String[] split = lookupSnapshotEntryStr.split("=");
+            String[] split = StringUtil.split(lookupSnapshotEntryStr, "=");
             lookupSnapshotMap.put(split[0], split[1]);
         }
         return lookupSnapshotMap;

diff --git a/engine-mr/src/main/java/org/apache/kylin/engine/mr/common/AbstractHadoopJob.java b/engine-mr/src/main/java/org/apache/kylin/engine/mr/common/AbstractHadoopJob.java
@@ -324,7 +324,7 @@ private void setJobTmpJarsAndFiles(Job job, String kylinDependency) {
             StringBuilder jarList = new StringBuilder();
             StringBuilder fileList = new StringBuilder();
 
-            for (String fileName : kylinDependency.split(",")) {
+            for (String fileName : StringUtil.splitAndTrim(kylinDependency, ",")) {
                 Path p = new Path(fileName);
                 if (p.isAbsolute() == false) {
                     logger.warn("The directory of kylin dependency '" + fileName + "' is not absolute, skip");

diff --git a/engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeDictionaryMapper.java b/engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeDictionaryMapper.java
@@ -42,6 +42,7 @@
 import org.apache.kylin.common.util.ByteArray;
 import org.apache.kylin.common.util.Bytes;
 import org.apache.kylin.common.util.HadoopUtil;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.cube.CubeDescManager;
 import org.apache.kylin.cube.CubeInstance;
 import org.apache.kylin.cube.CubeManager;
@@ -82,7 +83,7 @@ protected void doSetup(Context context) throws IOException, InterruptedException
         final CubeInstance cubeInstance = CubeManager.getInstance(kylinConfig).getCube(cubeName);
         final CubeDesc cubeDesc = CubeDescManager.getInstance(kylinConfig).getCubeDesc(cubeInstance.getDescName());
 
-        mergingSegments = getMergingSegments(cubeInstance, segmentIds.split(","));
+        mergingSegments = getMergingSegments(cubeInstance, StringUtil.splitByComma(segmentIds));
         tblColRefs = cubeDesc.getAllColumnsNeedDictionaryBuilt().toArray(new TblColRef[0]);
         dictMgr = DictionaryManager.getInstance(kylinConfig);
     }
@@ -114,11 +115,14 @@ protected void doMap(IntWritable key, NullWritable value, Context context)
 
         } else {
             // merge statistics
-            KylinConfig kylinConfig = AbstractHadoopJob.loadKylinConfigFromHdfs(new SerializableConfiguration(context.getConfiguration()), context.getConfiguration().get(BatchConstants.ARG_META_URL));
+            KylinConfig kylinConfig = AbstractHadoopJob.loadKylinConfigFromHdfs(
+                    new SerializableConfiguration(context.getConfiguration()),
+                    context.getConfiguration().get(BatchConstants.ARG_META_URL));
 
             final String cubeName = context.getConfiguration().get(BatchConstants.ARG_CUBE_NAME);
             final String segmentId = context.getConfiguration().get(BatchConstants.ARG_SEGMENT_ID);
-            final String statOutputPath = context.getConfiguration().get(MergeDictionaryJob.OPTION_OUTPUT_PATH_STAT.getOpt());
+            final String statOutputPath = context.getConfiguration()
+                    .get(MergeDictionaryJob.OPTION_OUTPUT_PATH_STAT.getOpt());
             CubeInstance cubeInstance = CubeManager.getInstance(kylinConfig).getCube(cubeName);
 
             logger.info("Statistics output path: {}", statOutputPath);
@@ -179,8 +183,10 @@ protected void doMap(IntWritable key, NullWritable value, Context context)
             }
 
             averageSamplingPercentage = averageSamplingPercentage / mergingSegments.size();
-            CubeStatsWriter.writeCuboidStatistics(conf, new Path(statOutputPath), cuboidHLLMap, averageSamplingPercentage);
-            Path statisticsFilePath = new Path(statOutputPath, BatchConstants.CFG_STATISTICS_CUBOID_ESTIMATION_FILENAME);
+            CubeStatsWriter.writeCuboidStatistics(conf, new Path(statOutputPath), cuboidHLLMap,
+                    averageSamplingPercentage);
+            Path statisticsFilePath = new Path(statOutputPath,
+                    BatchConstants.CFG_STATISTICS_CUBOID_ESTIMATION_FILENAME);
 
             FileSystem fs = HadoopUtil.getFileSystem(statisticsFilePath, conf);
             FSDataInputStream fis = fs.open(statisticsFilePath);

diff --git a/engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeDictionaryReducer.java b/engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/MergeDictionaryReducer.java
@@ -22,6 +22,7 @@
 
 import org.apache.hadoop.io.IntWritable;
 import org.apache.hadoop.io.Text;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.engine.mr.KylinReducer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -35,7 +36,7 @@ protected void doReduce(IntWritable key, Iterable<Text> values, Context context)
             throws IOException, InterruptedException {
         for (Text text : values) {
             String value = text.toString();
-            String[] splited = value.split("=");
+            String[] splited = StringUtil.split(value, "=");
             if (splited != null && splited.length == 2) {
                 logger.info("Dictionary for col {}, save at {}", splited[0], splited[1]);
                 context.write(new Text(splited[0]), new Text(splited[1]));

diff --git a/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkMergingDictionary.java b/engine-spark/src/main/java/org/apache/kylin/engine/spark/SparkMergingDictionary.java
@@ -41,6 +41,7 @@
 import org.apache.kylin.common.util.Bytes;
 import org.apache.kylin.common.util.HadoopUtil;
 import org.apache.kylin.common.util.OptionsHelper;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.cube.CubeDescManager;
 import org.apache.kylin.cube.CubeInstance;
 import org.apache.kylin.cube.CubeManager;
@@ -151,7 +152,7 @@ protected void execute(OptionsHelper optionsHelper) throws Exception {
             JavaRDD<Integer> indexRDD = sc.parallelize(indexs, columnLength + 1);
 
             JavaPairRDD<Text, Text> colToDictPathRDD = indexRDD.mapToPair(new MergeDictAndStatsFunction(cubeName,
-                    metaUrl, segmentId, segmentIds.split(","), statOutputPath, tblColRefs, sConf));
+                    metaUrl, segmentId, StringUtil.splitByComma(segmentIds), statOutputPath, tblColRefs, sConf));
 
             colToDictPathRDD.coalesce(1, false).saveAsNewAPIHadoopFile(dictOutputPath, Text.class, Text.class,
                     SequenceFileOutputFormat.class);

diff --git a/jdbc/src/main/java/org/apache/kylin/jdbc/KylinConnection.java b/jdbc/src/main/java/org/apache/kylin/jdbc/KylinConnection.java
@@ -27,6 +27,8 @@
 import java.util.Map;
 import java.util.Properties;
 
+import com.google.common.base.Splitter;
+import com.google.common.collect.Iterables;
 import org.apache.calcite.avatica.AvaticaConnection;
 import org.apache.calcite.avatica.AvaticaFactory;
 import org.apache.calcite.avatica.AvaticaParameter;
@@ -54,7 +56,7 @@ protected KylinConnection(UnregisteredDriver driver, JdbcFactory factory, String
         String odbcUrl = url;
         odbcUrl = odbcUrl.replaceAll((Driver.CONNECT_STRING_PREFIX + "[[A-Za-z0-9]*=[A-Za-z0-9]*;]*//").toString(), "");
 
-        String[] temps = odbcUrl.split("/");
+        String[] temps = Iterables.toArray(Splitter.on("/").split(odbcUrl), String.class);
         assert temps.length >= 2;
 
         this.project = temps[temps.length - 1];

diff --git a/kylin-it/src/test/java/org/apache/kylin/cube/cuboid/algorithm/ITAlgorithmTestBase.java b/kylin-it/src/test/java/org/apache/kylin/cube/cuboid/algorithm/ITAlgorithmTestBase.java
@@ -28,6 +28,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.cube.cuboid.TreeCuboidScheduler.CuboidCostComparator;
 import org.apache.kylin.cube.cuboid.TreeCuboidScheduler.CuboidTree;
 import org.junit.After;
@@ -94,7 +95,7 @@ protected Map<Long, Long> simulateCount() {
                     StandardCharsets.UTF_8));
 
             while ((sCurrentLine = br.readLine()) != null) {
-                String[] statPair = sCurrentLine.split(" ");
+                String[] statPair = StringUtil.split(sCurrentLine, " ");
                 countMap.put(Long.valueOf(statPair[0]), Long.valueOf(statPair[1]));
             }
 

diff --git a/kylin-it/src/test/java/org/apache/kylin/cube/inmemcubing/ITInMemCubeBuilderTest.java b/kylin-it/src/test/java/org/apache/kylin/cube/inmemcubing/ITInMemCubeBuilderTest.java
@@ -35,6 +35,7 @@
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.util.Dictionary;
 import org.apache.kylin.common.util.LocalFileMetadataTestCase;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.cube.CubeInstance;
 import org.apache.kylin.cube.CubeManager;
 import org.apache.kylin.cube.cuboid.Cuboid;
@@ -172,7 +173,7 @@ static void feedData(final CubeInstance cube, final String flatTable, ArrayBlock
         // get distinct values on each column
         List<String> lines = FileUtils.readLines(new File(flatTable), "UTF-8");
         for (String line : lines) {
-            String[] row = line.trim().split(",");
+            String[] row = StringUtil.splitByComma(line.trim());
             assert row.length == nColumns;
             for (int i = 0; i < nColumns; i++)
                 distinctSets[i].add(row[i]);
@@ -253,7 +254,7 @@ private static List<String> readValueList(String flatTable, int nColumns, int c)
         List<String> result = Lists.newArrayList();
         List<String> lines = FileUtils.readLines(new File(flatTable), "UTF-8");
         for (String line : lines) {
-            String[] row = line.trim().split(",");
+            String[] row = StringUtil.splitByComma(line.trim());
             if (row.length != nColumns) {
                 throw new IllegalStateException();
             }

diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPAuthentication.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPAuthentication.java
@@ -17,17 +17,19 @@
 */
 package org.apache.kylin.query.relnode;
 
+import org.apache.kylin.common.util.StringUtil;
+
 import java.util.ArrayList;
 
 /**
  * Created by wangcheng on 7/8/16.
  */
 public class OLAPAuthentication {
-     String username;
-     ArrayList<String> roles = new ArrayList<>();
+    String username;
+    ArrayList<String> roles = new ArrayList<>();
 
     public void parseUserInfo(String userInfo) {
-        String[] info = userInfo.split(",");
+        String[] info = StringUtil.splitByComma(userInfo);
         if (info.length > 0) //first element is username
             this.username = info[0];
         for (int i = 1; i < info.length; i++) //the remains should be roles which starts from index 1

diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java
@@ -68,6 +68,7 @@
 import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.metadata.model.ColumnDesc;
 import org.apache.kylin.metadata.model.DataModelDesc;
 import org.apache.kylin.metadata.model.TableRef;
@@ -239,7 +240,7 @@ private void modifyRules(List<String> rules, Function<RelOptRule, Void> func) {
             if (StringUtils.isEmpty(rule)) {
                 continue;
             }
-            String[] split = rule.split("#");
+            String[] split = StringUtil.split(rule, "#");
             if (split.length != 2) {
                 throw new RuntimeException("Customized Rule should be in format <RuleClassName>#<FieldName>");
             }

diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/TableController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/TableController.java
@@ -139,7 +139,7 @@ public Map<String, String[]> unLoadHiveTables(@PathVariable String tables, @Path
         Set<String> unLoadFail = Sets.newHashSet();
         Map<String, String[]> result = new HashMap<String, String[]>();
         try {
-            for (String tableName : tables.split(",")) {
+            for (String tableName : StringUtil.splitByComma(tables)) {
                 tableACLService.deleteFromTableACLByTbl(project, tableName);
                 if (tableService.unloadHiveTable(tableName, project)) {
                     unLoadSuccess.add(tableName);
@@ -169,7 +169,7 @@ public Map<String, String[]> unLoadHiveTables(@PathVariable String tables, @Path
     public CardinalityRequest generateCardinality(@PathVariable String tableNames,
             @RequestBody CardinalityRequest request, @PathVariable String project) throws Exception {
         String submitter = SecurityContextHolder.getContext().getAuthentication().getName();
-        String[] tables = tableNames.split(",");
+        String[] tables = StringUtil.splitByComma(tableNames);
         try {
             for (String table : tables) {
                 tableService.calculateCardinality(table.trim().toUpperCase(Locale.ROOT), submitter, project);

diff --git a/server-base/src/main/java/org/apache/kylin/rest/init/InitialTaskManager.java b/server-base/src/main/java/org/apache/kylin/rest/init/InitialTaskManager.java
@@ -20,6 +20,7 @@
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.rest.metrics.QueryMetrics2Facade;
 import org.apache.kylin.rest.metrics.QueryMetricsFacade;
 import org.slf4j.Logger;
@@ -49,7 +50,7 @@ private void runInitialTasks() {
         KylinConfig kylinConfig = KylinConfig.getInstanceFromEnv();
         String initTasks = kylinConfig.getInitTasks();
         if (!StringUtils.isEmpty(initTasks)) {
-            String[] taskClasses = initTasks.split(",");
+            String[] taskClasses = StringUtil.splitByComma(initTasks);
             for (String taskClass : taskClasses) {
                 try {
                     InitialTask task = (InitialTask) Class.forName(taskClass).newInstance();

diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AdminService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AdminService.java
@@ -32,6 +32,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.util.OrderedProperties;
+import org.apache.kylin.common.util.StringUtil;
 import org.apache.kylin.rest.constant.Constant;
 import org.apache.kylin.rest.job.StorageCleanupJob;
 import org.slf4j.Logger;
@@ -106,7 +107,7 @@ public String getPublicConfig() throws IOException {
 
         Collection<String> propertyKeys = Lists.newArrayList();
         if (StringUtils.isNotEmpty(whiteListProperties)) {
-            propertyKeys.addAll(Arrays.asList(whiteListProperties.split(",")));
+            propertyKeys.addAll(Arrays.asList(StringUtil.splitByComma(whiteListProperties)));
         }
 
         return KylinConfig.getInstanceFromEnv().exportToString(propertyKeys);