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 <[email protected]> Authored: Sun Feb 14 15:59:46 2016 +0800 Committer: Li, Yang <[email protected]> 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");
