You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by xx...@apache.org on 2023/05/06 06:59:30 UTC

[kylin] 29/38: KYLIN-5534 [FOLLOW UP] Set 'kylin.model.skip-check-tds' to true

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 <74...@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 <si...@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);