You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by li...@apache.org on 2016/02/14 09:00:09 UTC

[4/4] kylin git commit: KYLIN-1405 code review edits and other minor refactor

KYLIN-1405 code review edits and other minor refactor


Project: http://git-wip-us.apache.org/repos/asf/kylin/repo
Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/81ca14e9
Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/81ca14e9
Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/81ca14e9

Branch: refs/heads/2.x-staging
Commit: 81ca14e991e9a8d86970bb2a643aedd50859f6ff
Parents: e79e6b4
Author: Li, Yang <li...@apache.org>
Authored: Sun Feb 14 15:59:46 2016 +0800
Committer: Li, Yang <li...@apache.org>
Committed: Sun Feb 14 15:59:46 2016 +0800

----------------------------------------------------------------------
 .../apache/kylin/common/KylinConfigBase.java    | 12 ++++-
 .../kylin/common/persistence/ResourceTool.java  | 16 ++-----
 .../org/apache/kylin/cube/model/CubeDesc.java   | 48 ++++++++++----------
 .../validation/rule/AggregationGroupRule.java   |  3 +-
 .../org/apache/kylin/cube/CubeDescTest.java     | 15 +++---
 .../kylin/rest/security/CrossDomainFilter.java  |  2 +-
 6 files changed, 46 insertions(+), 50 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kylin/blob/81ca14e9/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
----------------------------------------------------------------------
diff --git a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
index 5ce4ddc..bd70249 100644
--- a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
+++ b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
@@ -73,11 +73,11 @@ public class KylinConfigBase implements Serializable {
 
     private volatile Properties properties = new Properties();
 
-    public String getOptional(String prop) {
+    protected String getOptional(String prop) {
         return getOptional(prop, null);
     }
 
-    public String getOptional(String prop, String dft) {
+    protected String getOptional(String prop, String dft) {
         final String property = System.getProperty(prop);
         return property != null ? property : properties.getProperty(prop, dft);
     }
@@ -375,6 +375,10 @@ public class KylinConfigBase implements Serializable {
     public double getCubeAlgorithmAutoThreshold() {
         return Double.parseDouble(getOptional("kylin.cube.algorithm.auto.threshold", "8"));
     }
+    
+    public int getCubeAggrGroupMaxSize() {
+        return Integer.parseInt(getOptional("kylin.cube.aggrgroup.max_size", "12"));
+    }
 
     public int getDictionaryMaxCardinality() {
         return Integer.parseInt(getOptional("kylin.dictionary.max.cardinality", "5000000"));
@@ -529,6 +533,10 @@ public class KylinConfigBase implements Serializable {
     public String getMailSender() {
         return getOptional("mail.sender", "");
     }
+    
+    public boolean isWebCrossDomainEnabled() {
+        return Boolean.parseBoolean(getOptional("crossdomain.enable", "true"));
+    }
 
     public String toString() {
         return getMetadataUrl();

http://git-wip-us.apache.org/repos/asf/kylin/blob/81ca14e9/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java
----------------------------------------------------------------------
diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java
index d1661ac..3b8e0c1 100644
--- a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java
+++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceTool.java
@@ -18,15 +18,14 @@
 
 package org.apache.kylin.common.persistence;
 
+import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.BufferedReader;
 import java.io.InputStreamReader;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.Callable;
 
-import com.google.common.base.Function;
+import org.apache.commons.io.IOUtils;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.util.StringUtil;
 
@@ -89,16 +88,9 @@ public class ResourceTool {
             while ((line = br.readLine()) != null) {
                 System.out.println(line);
             }
-        } catch (IOException e) {
-            e.printStackTrace();
         } finally {
-            if (br != null) {
-                try {
-                    br.close();
-                } catch (IOException e) {
-                    e.printStackTrace();
-                }
-            }
+            IOUtils.closeQuietly(is);
+            IOUtils.closeQuietly(br);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/kylin/blob/81ca14e9/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
----------------------------------------------------------------------
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java b/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
index be5e4ee..5a345b9 100644
--- a/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
@@ -20,8 +20,18 @@ package org.apache.kylin.cube.model;
 
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
+import java.util.TreeSet;
 
 import javax.annotation.Nullable;
 
@@ -35,9 +45,6 @@ import org.apache.kylin.common.persistence.RootPersistentEntity;
 import org.apache.kylin.common.util.Array;
 import org.apache.kylin.common.util.CaseInsensitiveStringMap;
 import org.apache.kylin.common.util.JsonUtil;
-import org.apache.kylin.cube.model.validation.IValidatorRule;
-import org.apache.kylin.cube.model.validation.ResultLevel;
-import org.apache.kylin.cube.model.validation.ValidateContext;
 import org.apache.kylin.measure.MeasureType;
 import org.apache.kylin.measure.extendedcolumn.ExtendedColumnMeasureType;
 import org.apache.kylin.metadata.MetadataConstants;
@@ -51,6 +58,8 @@ import org.apache.kylin.metadata.model.JoinDesc;
 import org.apache.kylin.metadata.model.MeasureDesc;
 import org.apache.kylin.metadata.model.TableDesc;
 import org.apache.kylin.metadata.model.TblColRef;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.annotation.JsonAutoDetect;
 import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
@@ -60,8 +69,6 @@ import com.google.common.base.Function;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  */
@@ -478,7 +485,7 @@ public class CubeDesc extends RootPersistentEntity {
         }
 
         // check if aggregation group is valid
-        validate(this);
+        validate();
 
         this.model = MetadataManager.getInstance(config).getDataModelDesc(this.modelName);
 
@@ -511,15 +518,11 @@ public class CubeDesc extends RootPersistentEntity {
         }
     }
 
-    public void validate(CubeDesc cube) {
-        inner(cube);
-    }
-
-    private void inner(CubeDesc cube) {
-        int maxSize = getMaxAgrGroupSize();
+    public void validate() {
+        int maxSize = config.getCubeAggrGroupMaxSize();
         int index = 0;
 
-        for (AggregationGroup agg : cube.getAggregationGroups()) {
+        for (AggregationGroup agg : getAggregationGroups()) {
             if (agg.getIncludes() == null) {
                 logger.error("Aggregation group " + index + " includes field not set");
                 throw new IllegalStateException("Aggregation group " + index + " includes field not set");
@@ -536,11 +539,11 @@ public class CubeDesc extends RootPersistentEntity {
             Set<String> mandatoryDims = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
             getDims(mandatoryDims, agg.getSelectRule().mandatory_dims);
 
-            ArrayList<Set<String>> hierarchyDimsList = new ArrayList ();
+            ArrayList<Set<String>> hierarchyDimsList = Lists.newArrayList();
             Set<String> hierarchyDims = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
             getDims(hierarchyDimsList, hierarchyDims, agg.getSelectRule().hierarchy_dims);
 
-            ArrayList<Set<String>> jointDimsList = new ArrayList ();
+            ArrayList<Set<String>> jointDimsList = Lists.newArrayList();
             Set<String> jointDims = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
             getDims(jointDimsList, jointDims, agg.getSelectRule().joint_dims);
 
@@ -595,7 +598,7 @@ public class CubeDesc extends RootPersistentEntity {
         }
     }
 
-    private void getDims (Set<String> dims, String [] stringSet) {
+    private void getDims(Set<String> dims, String[] stringSet) {
         if (stringSet != null) {
             for (String str : stringSet) {
                 dims.add(str);
@@ -603,7 +606,7 @@ public class CubeDesc extends RootPersistentEntity {
         }
     }
 
-    private void getDims (ArrayList<Set<String>> dimsList, Set<String> dims, String [][] stringSets) {
+    private void getDims(ArrayList<Set<String>> dimsList, Set<String> dims, String[][] stringSets) {
         Set<String> temp = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
         if (stringSets != null) {
             for (String[] ss : stringSets) {
@@ -626,7 +629,7 @@ public class CubeDesc extends RootPersistentEntity {
         return b;
     }
 
-    private boolean hasSingle (ArrayList<Set<String>> dimsList) {
+    private boolean hasSingle(ArrayList<Set<String>> dimsList) {
         boolean hasSingle = false;
         for (Set<String> dims : dimsList) {
             if (dims.size() < 2)
@@ -635,7 +638,7 @@ public class CubeDesc extends RootPersistentEntity {
         return hasSingle;
     }
 
-    private boolean hasOverlap (ArrayList<Set<String>> dimsList, Set<String> Dims) {
+    private boolean hasOverlap(ArrayList<Set<String>> dimsList, Set<String> Dims) {
         boolean hasOverlap = false;
         int dimSize = 0;
         for (Set<String> dims : dimsList) {
@@ -646,11 +649,6 @@ public class CubeDesc extends RootPersistentEntity {
         return hasOverlap;
     }
 
-    protected int getMaxAgrGroupSize() {
-        String size = KylinConfig.getInstanceFromEnv().getOptional(IValidatorRule.KEY_MAX_AGR_GROUP_SIZE, String.valueOf(IValidatorRule.DEFAULT_MAX_AGR_GROUP_SIZE));
-        return Integer.parseInt(size);
-    }
-
     private void initDimensionColumns() {
         for (DimensionDesc dim : dimensions) {
             JoinDesc join = dim.getJoin();

http://git-wip-us.apache.org/repos/asf/kylin/blob/81ca14e9/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java
----------------------------------------------------------------------
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java b/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java
index 222288a..196d2e8 100644
--- a/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java
@@ -179,7 +179,6 @@ public class AggregationGroupRule implements IValidatorRule<CubeDesc> {
     }
 
     protected int getMaxAgrGroupSize() {
-        String size = KylinConfig.getInstanceFromEnv().getOptional(KEY_MAX_AGR_GROUP_SIZE, String.valueOf(DEFAULT_MAX_AGR_GROUP_SIZE));
-        return Integer.parseInt(size);
+        return KylinConfig.getInstanceFromEnv().getCubeAggrGroupMaxSize();
     }
 }

http://git-wip-us.apache.org/repos/asf/kylin/blob/81ca14e9/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java
----------------------------------------------------------------------
diff --git a/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java b/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java
index 512f99f..7013cff 100644
--- a/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java
+++ b/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java
@@ -97,14 +97,13 @@ public class CubeDescTest extends LocalFileMetadataTestCase {
         thrown.expect(IllegalStateException.class);
         thrown.expectMessage("Aggregation group 0 has too many dimensions");
 
-        CubeDesc desc = new CubeDesc() {
-            @Override
-            protected int getMaxAgrGroupSize() {
-                return 3;
-            }
-        };
-        CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc");
-        desc.validate(cubeDesc);
+        CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc");
+        try {
+            System.setProperty("kylin.cube.aggrgroup.max_size", "3");
+            cubeDesc.validate();
+        } finally {
+            System.clearProperty("kylin.cube.aggrgroup.max_size");
+        }
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/kylin/blob/81ca14e9/server/src/main/java/org/apache/kylin/rest/security/CrossDomainFilter.java
----------------------------------------------------------------------
diff --git a/server/src/main/java/org/apache/kylin/rest/security/CrossDomainFilter.java b/server/src/main/java/org/apache/kylin/rest/security/CrossDomainFilter.java
index c9167b9..7d9d9ac 100644
--- a/server/src/main/java/org/apache/kylin/rest/security/CrossDomainFilter.java
+++ b/server/src/main/java/org/apache/kylin/rest/security/CrossDomainFilter.java
@@ -53,7 +53,7 @@ public class CrossDomainFilter implements Filter {
      */
     @Override
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
-        if (Boolean.parseBoolean(KylinConfig.getInstanceFromEnv().getOptional("crossdomain.enable", "true"))) {
+        if (KylinConfig.getInstanceFromEnv().isWebCrossDomainEnabled()) {
             ((HttpServletResponse) response).addHeader("Access-Control-Allow-Origin", "*");
             ((HttpServletResponse) response).addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
             ((HttpServletResponse) response).addHeader("Access-Control-Allow-Headers", "Origin, No-Cache, X-Requested-With, If-Modified-Since, Pragma, Last-Modified, Cache-Control, Expires, Content-Type, X-E4M-With, Accept, Authorization");