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/2622-2764 Commit: d873ca13aa67a90a99b66d99659ca571a3ab977f Parents: cd9a515 Author: Cheng Wang <cheng.w...@kyligence.io> Authored: Thu Aug 24 11:12:50 2017 +0800 Committer: Hongbin Ma <m...@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; + } }
