This is an automated email from the ASF dual-hosted git repository. xxyu pushed a commit to branch kylin5 in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 9af54c048448711da88658430af2bd147bc8989b Author: sibingzhang <74443791+sibingzh...@users.noreply.github.com> AuthorDate: Thu Mar 2 14:42:02 2023 +0800 KYLIN-5534 [FOLLOW UP] Set 'kylin.model.skip-check-tds' to true Co-authored-by: sibing.zhang <sibing.zh...@qq.com> --- .../org/apache/kylin/common/KylinConfigBase.java | 2 +- .../rest/controller/open/OpenModelController.java | 5 ++- .../apache/kylin/rest/service/ModelTdsService.java | 16 ++++---- .../service/ModelTdsServiceColumnNameTest.java | 3 +- .../kylin/rest/service/ModelTdsServiceTest.java | 45 ++++++++++++++-------- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java index 19021c4d73..fccc636084 100644 --- a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java +++ b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java @@ -3730,7 +3730,7 @@ public abstract class KylinConfigBase implements Serializable { } public boolean skipCheckTds() { - return Boolean.parseBoolean(getOptional("kylin.model.skip-check-tds", FALSE)); + return Boolean.parseBoolean(getOptional("kylin.model.skip-check-tds", TRUE)); } public boolean isHdfsMetricsPeriodicCalculationEnabled() { diff --git a/src/metadata-server/src/main/java/io/kyligence/kap/rest/controller/open/OpenModelController.java b/src/metadata-server/src/main/java/io/kyligence/kap/rest/controller/open/OpenModelController.java index b28319abeb..3b10bfc29a 100644 --- a/src/metadata-server/src/main/java/io/kyligence/kap/rest/controller/open/OpenModelController.java +++ b/src/metadata-server/src/main/java/io/kyligence/kap/rest/controller/open/OpenModelController.java @@ -73,6 +73,7 @@ import org.apache.kylin.rest.service.FusionIndexService; import org.apache.kylin.rest.service.FusionModelService; import org.apache.kylin.rest.service.ModelService; import org.apache.kylin.rest.service.ModelTdsService; +import org.apache.kylin.rest.util.AclPermissionUtil; import org.apache.kylin.tool.bisync.SyncContext; import org.apache.kylin.tool.bisync.model.SyncModel; import org.apache.kylin.util.DataRangeUtils; @@ -413,7 +414,9 @@ public class OpenModelController extends NBasicController { } SyncContext syncContext = tdsService.prepareSyncContext(projectName, modelId, exportAs, element, host, port); - SyncModel syncModel = tdsService.exportModel(syncContext); + SyncModel syncModel = AclPermissionUtil.isAdmin() + ? tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, dimensions, measures) + : tdsService.exportTDSDimensionsAndMeasuresByNormalUser(syncContext, dimensions, measures); tdsService.preCheckNameConflict(syncModel); tdsService.dumpSyncModel(syncContext, syncModel, response); } diff --git a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelTdsService.java b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelTdsService.java index 84e611fb96..847d22905c 100644 --- a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelTdsService.java +++ b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelTdsService.java @@ -129,9 +129,15 @@ public class ModelTdsService extends AbstractModelService { } public SyncModel exportModel(SyncContext syncContext) { - return AclPermissionUtil.isAdmin() - ? exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), ImmutableList.of()) - : exportTDSDimensionsAndMeasuresByNormalUser(syncContext, ImmutableList.of(), ImmutableList.of()); + if (AclPermissionUtil.isAdmin()) { + return exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), ImmutableList.of()); + } + + String projectName = syncContext.getProjectName(); + String modelId = syncContext.getModelId(); + checkModelExportPermission(projectName, modelId); + checkModelPermission(projectName, modelId); + return exportTDSDimensionsAndMeasuresByNormalUser(syncContext, ImmutableList.of(), ImmutableList.of()); } public SyncModel exportTDSDimensionsAndMeasuresByNormalUser(SyncContext syncContext, List<String> dimensions, @@ -168,10 +174,6 @@ public class ModelTdsService extends AbstractModelService { public SyncModel exportTDSDimensionsAndMeasuresByAdmin(SyncContext syncContext, List<String> dimensions, List<String> measures) { - String projectName = syncContext.getProjectName(); - String modelId = syncContext.getModelId(); - checkModelExportPermission(projectName, modelId); - checkModelPermission(projectName, modelId); return new SyncModelBuilder(syncContext).buildSourceSyncModel(dimensions, measures); } diff --git a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceColumnNameTest.java b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceColumnNameTest.java index 1047372feb..6131b84e70 100644 --- a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceColumnNameTest.java +++ b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceColumnNameTest.java @@ -123,7 +123,8 @@ public class ModelTdsServiceColumnNameTest extends SourceTestCase { syncContext.setAdmin(true); syncContext.setDataflow(NDataflowManager.getInstance(getTestConfig(), getProject()).getDataflow(modelId)); syncContext.setKylinConfig(getTestConfig()); - SyncModel syncModel = tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), ImmutableList.of()); + SyncModel syncModel = tdsService.exportModel(syncContext); + overwriteSystemProp("kylin.model.skip-check-tds", "false"); Assert.assertTrue(tdsService.preCheckNameConflict(syncModel)); } } diff --git a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceTest.java b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceTest.java index 885408a725..03f6fa39c3 100644 --- a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceTest.java +++ b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelTdsServiceTest.java @@ -161,21 +161,34 @@ public class ModelTdsServiceTest extends SourceTestCase { syncContext.setAdmin(true); syncContext.setDataflow(NDataflowManager.getInstance(getTestConfig(), projectName).getDataflow(modelId)); syncContext.setKylinConfig(getTestConfig()); - SyncModel syncModel = tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), - ImmutableList.of()); + SyncModel syncModel = tdsService.exportModel(syncContext); + overwriteSystemProp("kylin.model.skip-check-tds", "false"); Assert.assertThrows( "There are duplicated names among dimension column LO_LINENUMBER and measure name LO_LINENUMBER. Cannot export a valid TDS file. Please correct the duplicated names and try again.", KylinException.class, () -> tdsService.preCheckNameConflict(syncModel)); syncContext.setAdmin(false); prepareBasicPermissionByModel(projectName, syncContext.getDataflow().getModel()); + Mockito.when(accessService.getGroupsOfExecuteUser(Mockito.any(String.class))) + .thenReturn(Sets.newHashSet("ROLE_ANALYST")); SecurityContextHolder.getContext() .setAuthentication(new TestingAuthenticationToken("u1", "ANALYST", Constant.ROLE_ANALYST)); - SyncModel syncModel2 = tdsService.exportTDSDimensionsAndMeasuresByNormalUser(syncContext, ImmutableList.of(), - ImmutableList.of()); + SyncModel syncModel2 = tdsService.exportModel(syncContext); Assert.assertThrows( "There are duplicated names among dimension column LO_LINENUMBER and measure name LO_LINENUMBER. Cannot export a valid TDS file. Please correct the duplicated names and try again.", KylinException.class, () -> tdsService.preCheckNameConflict(syncModel2)); + + overwriteSystemProp("kylin.model.skip-check-tds", "true"); + Assert.assertTrue(tdsService.preCheckNameConflict(syncModel)); + + syncContext.setAdmin(false); + prepareBasicPermissionByModel(projectName, syncContext.getDataflow().getModel()); + Mockito.when(accessService.getGroupsOfExecuteUser(Mockito.any(String.class))) + .thenReturn(Sets.newHashSet("ROLE_ANALYST")); + SecurityContextHolder.getContext() + .setAuthentication(new TestingAuthenticationToken("u1", "ANALYST", Constant.ROLE_ANALYST)); + SyncModel syncModel3 = tdsService.exportModel(syncContext); + Assert.assertTrue(tdsService.preCheckNameConflict(syncModel3)); } @Test @@ -199,16 +212,16 @@ public class ModelTdsServiceTest extends SourceTestCase { syncContext.setAdmin(true); syncContext.setDataflow(NDataflowManager.getInstance(getTestConfig(), projectName).getDataflow(modelId)); syncContext.setKylinConfig(getTestConfig()); - SyncModel syncModel = tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), - ImmutableList.of()); + SyncModel syncModel = tdsService.exportModel(syncContext); Assert.assertTrue(tdsService.preCheckNameConflict(syncModel)); syncContext.setAdmin(false); prepareBasicPermissionByModel(projectName, syncContext.getDataflow().getModel()); + Mockito.when(accessService.getGroupsOfExecuteUser(Mockito.any(String.class))) + .thenReturn(Sets.newHashSet("ROLE_ANALYST")); SecurityContextHolder.getContext() .setAuthentication(new TestingAuthenticationToken("u1", "ANALYST", Constant.ROLE_ANALYST)); - syncModel = tdsService.exportTDSDimensionsAndMeasuresByNormalUser(syncContext, ImmutableList.of(), - ImmutableList.of()); + syncModel = tdsService.exportModel(syncContext); Assert.assertTrue(tdsService.preCheckNameConflict(syncModel)); } @@ -233,18 +246,19 @@ public class ModelTdsServiceTest extends SourceTestCase { syncContext.setDataflow(NDataflowManager.getInstance(getTestConfig(), projectName).getDataflow(modelId)); syncContext.setKylinConfig(getTestConfig()); syncContext.setAdmin(true); - SyncModel syncModel = tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), - ImmutableList.of()); + overwriteSystemProp("kylin.model.skip-check-tds", "false"); + SyncModel syncModel = tdsService.exportModel(syncContext); Assert.assertThrows( "There are duplicated names among model column LO_LINENUMBER and measure name LO_LINENUMBER. Cannot export a valid TDS file. Please correct the duplicated names and try again.", KylinException.class, () -> tdsService.preCheckNameConflict(syncModel)); syncContext.setAdmin(false); prepareBasicPermissionByModel(projectName, syncContext.getDataflow().getModel()); + Mockito.when(accessService.getGroupsOfExecuteUser(Mockito.any(String.class))) + .thenReturn(Sets.newHashSet("ROLE_ANALYST")); SecurityContextHolder.getContext() .setAuthentication(new TestingAuthenticationToken("u1", "ANALYST", Constant.ROLE_ANALYST)); - SyncModel syncModel2 = tdsService.exportTDSDimensionsAndMeasuresByNormalUser(syncContext, ImmutableList.of(), - ImmutableList.of()); + SyncModel syncModel2 = tdsService.exportModel(syncContext); Assert.assertThrows( "There are duplicated names among model column LO_LINENUMBER and measure name LO_LINENUMBER. Cannot export a valid TDS file. Please correct the duplicated names and try again.", KylinException.class, () -> tdsService.preCheckNameConflict(syncModel2)); @@ -344,7 +358,7 @@ public class ModelTdsServiceTest extends SourceTestCase { thrown.expectMessage("current user does not have full permission on requesting model"); SyncContext syncContext = tdsService.prepareSyncContext(project, modelId, SyncContext.BI.TABLEAU_CONNECTOR_TDS, SyncContext.ModelElement.AGG_INDEX_COL, "localhost", 8080); - tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), ImmutableList.of()); + tdsService.exportModel(syncContext); } finally { SecurityContextHolder.getContext() .setAuthentication(new TestingAuthenticationToken("ADMIN", "ADMIN", Constant.ROLE_ADMIN)); @@ -393,7 +407,7 @@ public class ModelTdsServiceTest extends SourceTestCase { SyncContext syncContext = tdsService.prepareSyncContext(project, modelId, SyncContext.BI.TABLEAU_CONNECTOR_TDS, SyncContext.ModelElement.AGG_INDEX_COL, "localhost", 8080); - tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), ImmutableList.of()); + tdsService.exportModel(syncContext); } finally { SecurityContextHolder.getContext() .setAuthentication(new TestingAuthenticationToken("ADMIN", "ADMIN", Constant.ROLE_ADMIN)); @@ -483,8 +497,7 @@ public class ModelTdsServiceTest extends SourceTestCase { prepareBasic(project); SyncContext syncContext = tdsService.prepareSyncContext(project, modelId, SyncContext.BI.TABLEAU_CONNECTOR_TDS, SyncContext.ModelElement.AGG_INDEX_AND_TABLE_INDEX_COL, "localhost", 7070); - SyncModel syncModel = tdsService.exportTDSDimensionsAndMeasuresByAdmin(syncContext, ImmutableList.of(), - ImmutableList.of()); + SyncModel syncModel = tdsService.exportModel(syncContext); TableauDatasourceModel datasource1 = (TableauDatasourceModel) BISyncTool.getBISyncModel(syncContext, syncModel); ByteArrayOutputStream outStream4 = new ByteArrayOutputStream(); datasource1.dump(outStream4);