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 2017/09/01 02:55:03 UTC
[27/50] [abbrv] kylin git commit: KYLIN-2807,
enhance validate of datamodel's update
KYLIN-2807, enhance validate of datamodel's update
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo
Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/d873ca13
Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/d873ca13
Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/d873ca13
Branch: refs/heads/master
Commit: d873ca13aa67a90a99b66d99659ca571a3ab977f
Parents: cd9a515
Author: Cheng Wang <ch...@kyligence.io>
Authored: Thu Aug 24 11:12:50 2017 +0800
Committer: Hongbin Ma <ma...@kyligence.io>
Committed: Thu Aug 24 11:27:44 2017 +0800
----------------------------------------------------------------------
.../apache/kylin/rest/service/ModelService.java | 123 ++++++++++++++-----
.../kylin/rest/service/ModelServiceTest.java | 94 +++++++++++++-
2 files changed, 179 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kylin/blob/d873ca13/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java
----------------------------------------------------------------------
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java
index 14b2280..82632eb 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java
@@ -22,6 +22,7 @@ import static org.apache.kylin.rest.controller2.ModelControllerV2.VALID_MODELNAM
import java.io.IOException;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -299,55 +300,112 @@ public class ModelService extends BasicService {
return true;
}
- private boolean checkIfBreakExistingCubes(DataModelDesc dataModelDesc, String project) throws IOException {
+ private List<String> getModelCols(DataModelDesc model) {
+ List<String> dimCols = new ArrayList<String>();
+
+ List<ModelDimensionDesc> dimensions = model.getDimensions();
+
+ for (ModelDimensionDesc dim : dimensions) {
+ String table = dim.getTable();
+ for (String c : dim.getColumns()) {
+ dimCols.add(table + "." + c);
+ }
+ }
+ return dimCols;
+ }
+
+ private List<String> getModelMeasures(DataModelDesc model) {
+ List<String> measures = new ArrayList<String>();
+
+ for (String s : model.getMetrics()) {
+ measures.add(s);
+ }
+ return measures;
+ }
+
+ private Map<String, List<String>> getInfluencedCubesByDims(List<String> dims, List<CubeInstance> cubes) {
+ Map<String, List<String>> influencedCubes = new HashMap<>();
+ for (CubeInstance cubeInstance : cubes) {
+ CubeDesc cubeDesc = cubeInstance.getDescriptor();
+ for (TblColRef tblColRef : cubeDesc.listDimensionColumnsIncludingDerived()) {
+ if (dims.contains(tblColRef.getIdentity()))
+ continue;
+ if (influencedCubes.get(tblColRef.getIdentity()) == null) {
+ List<String> candidates = new ArrayList<>();
+ candidates.add(cubeInstance.getName());
+ influencedCubes.put(tblColRef.getIdentity(), candidates);
+ } else
+ influencedCubes.get(tblColRef.getIdentity()).add(cubeInstance.getName());
+ }
+ }
+ return influencedCubes;
+ }
+
+ private Map<String, List<String>> getInfluencedCubesByMeasures(List<String> allCols, List<CubeInstance> cubes) {
+ Map<String, List<String>> influencedCubes = new HashMap<>();
+ for (CubeInstance cubeInstance : cubes) {
+ CubeDesc cubeDesc = cubeInstance.getDescriptor();
+ Set<TblColRef> tblColRefs = Sets.newHashSet(cubeDesc.listAllColumns());
+ tblColRefs.removeAll(cubeDesc.listDimensionColumnsIncludingDerived());
+ for (TblColRef tblColRef : tblColRefs) {
+ if (allCols.contains(tblColRef.getIdentity()))
+ continue;
+ if (influencedCubes.get(tblColRef.getIdentity()) == null) {
+ List<String> candidates = new ArrayList<>();
+ candidates.add(cubeInstance.getName());
+ influencedCubes.put(tblColRef.getIdentity(), candidates);
+ } else
+ influencedCubes.get(tblColRef.getIdentity()).add(cubeInstance.getName());
+ }
+ }
+ return influencedCubes;
+ }
+
+ private String checkIfBreakExistingCubes(DataModelDesc dataModelDesc, String project) throws IOException {
String modelName = dataModelDesc.getName();
List<CubeInstance> cubes = cubeService.listAllCubes(null, project, modelName, true);
+ DataModelDesc originDataModelDesc = listAllModels(modelName, project, true).get(0);
+ StringBuilder checkRet = new StringBuilder();
if (cubes != null && cubes.size() != 0) {
dataModelDesc.init(getConfig(), getMetadataManager().getAllTablesMap(dataModelDesc.getProject()),
getMetadataManager().listDataModels());
- List<String> dimCols = new ArrayList<String>();
- List<String> dimAndMCols = new ArrayList<String>();
+ List<String> curModelDims = getModelCols(dataModelDesc);
+ List<String> curModelMeasures = getModelMeasures(dataModelDesc);
- List<ModelDimensionDesc> dimensions = dataModelDesc.getDimensions();
- String[] measures = dataModelDesc.getMetrics();
+ List<String> curModelDimsAndMeasures = new ArrayList<>();
+ curModelDimsAndMeasures.addAll(curModelDims);
+ curModelDimsAndMeasures.addAll(curModelMeasures);
- for (ModelDimensionDesc dim : dimensions) {
- String table = dim.getTable();
- for (String c : dim.getColumns()) {
- dimCols.add(table + "." + c);
- }
- }
+ Map<String, List<String>> influencedCubesByDims = getInfluencedCubesByDims(curModelDims, cubes);
+ Map<String, List<String>> influencedCubesByMeasures = getInfluencedCubesByMeasures(curModelDimsAndMeasures,
+ cubes);
- dimAndMCols.addAll(dimCols);
-
- for (String measure : measures) {
- dimAndMCols.add(measure);
+ for (Map.Entry<String, List<String>> e : influencedCubesByDims.entrySet()) {
+ checkRet.append("Dimension: ");
+ checkRet.append(e.getKey());
+ checkRet.append(" can't be removed, It is referred in Cubes: ");
+ checkRet.append(e.getValue().toString());
+ checkRet.append("\r\n");
}
- Set<TblColRef> usedDimCols = getUsedDimCols(modelName, project).keySet();
- Set<TblColRef> usedNonDimCols = getUsedNonDimCols(modelName, project).keySet();
-
- for (TblColRef tblColRef : usedDimCols) {
- if (!dimCols.contains(tblColRef.getTableAlias() + "." + tblColRef.getName()))
- return false;
+ for (Map.Entry<String, List<String>> e : influencedCubesByMeasures.entrySet()) {
+ checkRet.append("Measure: ");
+ checkRet.append(e.getKey());
+ checkRet.append(" can't be removed, It is referred in Cubes: ");
+ checkRet.append(e.getValue().toString());
+ checkRet.append("\r\n");
}
- for (TblColRef tblColRef : usedNonDimCols) {
- if (!dimAndMCols.contains(tblColRef.getTableAlias() + "." + tblColRef.getName()))
- return false;
- }
-
- DataModelDesc originDataModelDesc = listAllModels(modelName, project, true).get(0);
if (!dataModelDesc.getRootFactTable().equals(originDataModelDesc.getRootFactTable()))
- return false;
+ checkRet.append("Root fact table can't be modified. \r\n");
JoinsTree joinsTree = dataModelDesc.getJoinsTree(), originJoinsTree = originDataModelDesc.getJoinsTree();
if (joinsTree.matchNum(originJoinsTree) != originDataModelDesc.getJoinTables().length + 1)
- return false;
+ checkRet.append("The join shouldn't be modified in this model.");
}
- return true;
+ return checkRet.toString();
}
public void primaryCheck(DataModelDesc modelDesc) {
@@ -383,8 +441,9 @@ public class ModelService extends BasicService {
modelDesc = createModelDesc(projectName, modelDesc);
} else {
// update
- if (!checkIfBreakExistingCubes(modelDesc, projectName)) {
- throw new BadRequestException(msg.getUPDATE_MODEL_KEY_FIELD());
+ String error = checkIfBreakExistingCubes(modelDesc, projectName);
+ if (!error.isEmpty()) {
+ throw new BadRequestException(error);
}
modelDesc = updateModelAndDesc(modelDesc);
}
http://git-wip-us.apache.org/repos/asf/kylin/blob/d873ca13/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
----------------------------------------------------------------------
diff --git a/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
index 78b1979..b51d90c 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
@@ -30,6 +30,7 @@ import org.apache.kylin.job.exception.JobException;
import org.apache.kylin.metadata.MetadataManager;
import org.apache.kylin.metadata.model.ComputedColumnDesc;
import org.apache.kylin.metadata.model.DataModelDesc;
+import org.apache.kylin.metadata.model.ModelDimensionDesc;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
@@ -62,7 +63,8 @@ public class ModelServiceTest extends ServiceTestBase {
}
@Test
- public void testSuccessModelUpdateOnComputedColumn() throws IOException, JobException, NoSuchFieldException, IllegalAccessException {
+ public void testSuccessModelUpdateOnComputedColumn()
+ throws IOException, JobException, NoSuchFieldException, IllegalAccessException {
List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true);
Assert.assertTrue(dataModelDescs.size() == 1);
@@ -79,9 +81,11 @@ public class ModelServiceTest extends ServiceTestBase {
}
@Test
- public void testFailureModelUpdateDueToComputedColumnConflict() throws IOException, JobException, NoSuchFieldException, IllegalAccessException {
+ public void testFailureModelUpdateDueToComputedColumnConflict()
+ throws IOException, JobException, NoSuchFieldException, IllegalAccessException {
expectedEx.expect(IllegalArgumentException.class);
- expectedEx.expectMessage("Column name for computed column DEFAULT.TEST_KYLIN_FACT.DEAL_AMOUNT is already used in model ci_inner_join_model, you should apply the same expression ' PRICE * ITEM_COUNT ' here, or use a different column name.");
+ expectedEx.expectMessage(
+ "Column name for computed column DEFAULT.TEST_KYLIN_FACT.DEAL_AMOUNT is already used in model ci_inner_join_model, you should apply the same expression ' PRICE * ITEM_COUNT ' here, or use a different column name.");
List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true);
Assert.assertTrue(dataModelDescs.size() == 1);
@@ -97,11 +101,12 @@ public class ModelServiceTest extends ServiceTestBase {
DataModelDesc dataModelDesc = modelService.updateModelAndDesc(deserialize);
}
-
@Test
- public void testFailureModelUpdateDueToComputedColumnConflict2() throws IOException, JobException, NoSuchFieldException, IllegalAccessException {
+ public void testFailureModelUpdateDueToComputedColumnConflict2()
+ throws IOException, JobException, NoSuchFieldException, IllegalAccessException {
expectedEx.expect(IllegalArgumentException.class);
- expectedEx.expectMessage("There is already a column named cal_dt on table DEFAULT.TEST_KYLIN_FACT, please change your computed column name");
+ expectedEx.expectMessage(
+ "There is already a column named cal_dt on table DEFAULT.TEST_KYLIN_FACT, please change your computed column name");
List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true);
Assert.assertTrue(dataModelDescs.size() == 1);
@@ -116,4 +121,81 @@ public class ModelServiceTest extends ServiceTestBase {
field.set(deserialize.getComputedColumnDescs().get(0), "cal_dt");
DataModelDesc dataModelDesc = modelService.updateModelAndDesc(deserialize);
}
+
+ @Test
+ public void testRevisableModelInCaseOfDeleteMeasure() throws IOException {
+ List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true);
+ Assert.assertTrue(dataModelDescs.size() == 1);
+
+ DataModelDesc revisableModel = dataModelDescs.get(0);
+
+ String[] originM = revisableModel.getMetrics();
+ String[] reviseM = cutItems(originM, 1);
+
+ revisableModel.setMetrics(reviseM);
+
+ expectedEx.expect(org.apache.kylin.rest.exception.BadRequestException.class);
+ expectedEx.expectMessage(
+ "Measure: TEST_KYLIN_FACT.DEAL_AMOUNT can't be removed, It is referred in Cubes: [ci_left_join_cube]");
+ modelService.updateModelToResourceStore(revisableModel, "default");
+ }
+
+ @Test
+ public void testRevisableModelInCaseOfDeleteDims() throws IOException {
+ List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true);
+ Assert.assertTrue(dataModelDescs.size() == 1);
+
+ DataModelDesc revisableModel = dataModelDescs.get(0);
+
+ List<ModelDimensionDesc> originDims = revisableModel.getDimensions();
+ String[] reviseDims = cutItems(originDims.get(0).getColumns(), 2);
+ originDims.get(0).setColumns(reviseDims);
+ revisableModel.setDimensions(originDims);
+
+ expectedEx.expect(org.apache.kylin.rest.exception.BadRequestException.class);
+ expectedEx.expectMessage(
+ "Dimension: TEST_KYLIN_FACT.SELLER_ID_AND_COUNTRY_NAME can't be removed, It is referred in Cubes: [ci_left_join_cube]");
+ modelService.updateModelToResourceStore(revisableModel, "default");
+ }
+
+ @Test
+ public void testRevisableModelInCaseOfMoveMeasureToDim() throws IOException {
+ List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true);
+ Assert.assertTrue(dataModelDescs.size() == 1);
+
+ DataModelDesc revisableModel = dataModelDescs.get(0);
+
+ String[] originM = revisableModel.getMetrics();
+ String[] reviseM = cutItems(originM, 1);
+ revisableModel.setMetrics(reviseM);
+
+ String col = originM[originM.length - 1];
+ col = col.substring(col.indexOf('.') + 1);
+
+ List<ModelDimensionDesc> originDims = revisableModel.getDimensions();
+ String[] reviseDims = addItems(originDims.get(0).getColumns(), col);
+ originDims.get(0).setColumns(reviseDims);
+ revisableModel.setDimensions(originDims);
+ modelService.updateModelToResourceStore(revisableModel, "default");
+ //It should pass without any exceptions.
+ }
+
+ private String[] cutItems(String[] origin, int count) {
+ if (origin == null)
+ return null;
+
+ String[] ret = new String[origin.length - count];
+ for (int i = 0; i < ret.length; i++)
+ ret[i] = origin[i];
+ return ret;
+ }
+
+ private String[] addItems(String[] origin, String... toAddItems) {
+ if (origin == null)
+ return null;
+ String[] ret = new String[origin.length + toAddItems.length];
+ System.arraycopy(origin, 0, ret, 0, origin.length);
+ System.arraycopy(toAddItems, 0, ret, origin.length, toAddItems.length);
+ return ret;
+ }
}