From 4441d91dcf178653e55b94fe564c8c9907e897fa Mon Sep 17 00:00:00 2001
From: lgcareer <18610854716@163.com>
Date: Wed, 3 Jun 2020 13:50:06 +0800
Subject: [PATCH] Fix bug:If user didn't set the value of tenant,release
 process definition fail (#2885)

* copy resources need get top directory first

* copy resources need get top directory first

* fix #2860:copy files and create empty directory

* Fix bug:If user didn't set the value of tenant,release process definition online fail
---
 .../api/service/ResourcesService.java         | 21 +++++++++++++++----
 .../api/service/ResourcesServiceTest.java     | 14 ++++++-------
 .../dao/mapper/UserMapper.xml                 |  6 ++++--
 .../service/permission/PermissionCheck.java   |  4 ++++
 .../service/process/ProcessService.java       |  2 +-
 5 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
index 18ffa5050c..173856b513 100644
--- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
+++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
@@ -1005,8 +1005,21 @@ public class ResourcesService extends BaseService {
             logger.error("resource id {} is directory,can't download it", resourceId);
             throw new RuntimeException("cant't download directory");
         }
-        User user = userMapper.queryDetailsById(resource.getUserId());
-        String tenantCode = tenantMapper.queryById(user.getTenantId()).getTenantCode();
+
+        int userId = resource.getUserId();
+        User user = userMapper.selectById(userId);
+        if(user == null){
+            logger.error("user id {} not exists", userId);
+            throw new RuntimeException(String.format("resource owner id %d not exist",userId));
+        }
+
+        Tenant tenant = tenantMapper.queryById(user.getTenantId());
+        if(tenant == null){
+            logger.error("tenant id {} not exists", user.getTenantId());
+            throw new RuntimeException(String.format("The tenant id %d of resource owner not exist",user.getTenantId()));
+        }
+
+        String tenantCode = tenant.getTenantCode();
 
         String hdfsFileName = HadoopUtils.getHdfsFileName(resource.getType(), tenantCode, resource.getAlias());
 
@@ -1172,8 +1185,8 @@ public class ResourcesService extends BaseService {
      */
     private String getTenantCode(int userId,Result result){
 
-        User user = userMapper.queryDetailsById(userId);
-        if(user == null){
+        User user = userMapper.selectById(userId);
+        if (user == null) {
             logger.error("user {} not exists", userId);
             putMsg(result, Status.USER_NOT_EXIST,userId);
             return null;
diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java
index e52f4670e2..407f6b587f 100644
--- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java
+++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java
@@ -177,7 +177,7 @@ public class ResourcesServiceTest {
 
         //RESOURCE_NOT_EXIST
         user.setId(1);
-        Mockito.when(userMapper.queryDetailsById(1)).thenReturn(getUser());
+        Mockito.when(userMapper.selectById(1)).thenReturn(getUser());
         Mockito.when(tenantMapper.queryById(1)).thenReturn(getTenant());
         PowerMockito.when(HadoopUtils.getHdfsFileName(Mockito.any(), Mockito.any(),Mockito.anyString())).thenReturn("test1");
 
@@ -209,13 +209,13 @@ public class ResourcesServiceTest {
         logger.info(result.toString());
         Assert.assertEquals(Status.RESOURCE_EXIST.getMsg(),result.getMsg());
         //USER_NOT_EXIST
-        Mockito.when(userMapper.queryDetailsById(Mockito.anyInt())).thenReturn(null);
+        Mockito.when(userMapper.selectById(Mockito.anyInt())).thenReturn(null);
         result = resourcesService.updateResource(user,1,"ResourcesServiceTest1.jar","ResourcesServiceTest",ResourceType.UDF);
         logger.info(result.toString());
         Assert.assertTrue(Status.USER_NOT_EXIST.getCode() == result.getCode());
 
         //TENANT_NOT_EXIST
-        Mockito.when(userMapper.queryDetailsById(1)).thenReturn(getUser());
+        Mockito.when(userMapper.selectById(1)).thenReturn(getUser());
         Mockito.when(tenantMapper.queryById(Mockito.anyInt())).thenReturn(null);
         result = resourcesService.updateResource(user,1,"ResourcesServiceTest1.jar","ResourcesServiceTest",ResourceType.UDF);
         logger.info(result.toString());
@@ -296,7 +296,7 @@ public class ResourcesServiceTest {
             //TENANT_NOT_EXIST
             loginUser.setUserType(UserType.ADMIN_USER);
             loginUser.setTenantId(2);
-            Mockito.when(userMapper.queryDetailsById(Mockito.anyInt())).thenReturn(loginUser);
+            Mockito.when(userMapper.selectById(Mockito.anyInt())).thenReturn(loginUser);
             result = resourcesService.delete(loginUser,1);
             logger.info(result.toString());
             Assert.assertEquals(Status.TENANT_NOT_EXIST.getMsg(), result.getMsg());
@@ -390,7 +390,7 @@ public class ResourcesServiceTest {
 
 
         //TENANT_NOT_EXIST
-        Mockito.when(userMapper.queryDetailsById(1)).thenReturn(getUser());
+        Mockito.when(userMapper.selectById(1)).thenReturn(getUser());
         result = resourcesService.readResource(1,1,10);
         logger.info(result.toString());
         Assert.assertEquals(Status.TENANT_NOT_EXIST.getMsg(),result.getMsg());
@@ -495,7 +495,7 @@ public class ResourcesServiceTest {
 
 
         //TENANT_NOT_EXIST
-        Mockito.when(userMapper.queryDetailsById(1)).thenReturn(getUser());
+        Mockito.when(userMapper.selectById(1)).thenReturn(getUser());
         result = resourcesService.updateResourceContent(1,"content");
         logger.info(result.toString());
         Assert.assertTrue(Status.TENANT_NOT_EXIST.getCode() == result.getCode());
@@ -514,7 +514,7 @@ public class ResourcesServiceTest {
 
         PowerMockito.when(PropertyUtils.getResUploadStartupState()).thenReturn(true);
         Mockito.when(tenantMapper.queryById(1)).thenReturn(getTenant());
-        Mockito.when(userMapper.queryDetailsById(1)).thenReturn(getUser());
+        Mockito.when(userMapper.selectById(1)).thenReturn(getUser());
         org.springframework.core.io.Resource resourceMock = Mockito.mock(org.springframework.core.io.Resource.class);
         try {
             //resource null
diff --git a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UserMapper.xml b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UserMapper.xml
index fcf8a137e6..003a412178 100644
--- a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UserMapper.xml
+++ b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UserMapper.xml
@@ -46,8 +46,10 @@
     <select id="queryDetailsById" resultType="org.apache.dolphinscheduler.dao.entity.User">
         select u.*, t.tenant_name,
         case when u.queue <![CDATA[ <> ]]>  '' then u.queue else q.queue_name end as queue_name
-        from t_ds_user u,t_ds_tenant t,t_ds_queue q
-        WHERE u.tenant_id = t.id and t.queue_id = q.id and u.id = #{userId}
+        from t_ds_user u
+        left join t_ds_tenant t on u.tenant_id=t.id
+        left join t_ds_queue q on t.queue_id = q.id
+        WHERE u.id = #{userId}
     </select>
     <select id="queryUserListByAlertGroupId" resultType="org.apache.dolphinscheduler.dao.entity.User">
       select u.*
diff --git a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/permission/PermissionCheck.java b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/permission/PermissionCheck.java
index 9f93f4ce3e..1a9295bb10 100644
--- a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/permission/PermissionCheck.java
+++ b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/permission/PermissionCheck.java
@@ -174,6 +174,10 @@ public class PermissionCheck<T> {
 
             // get user type in order to judge whether the user is admin
             User user = processService.getUserById(userId);
+            if (user == null) {
+                logger.error("user id {} didn't exist",userId);
+                throw new RuntimeException(String.format("user %s didn't exist",userId));
+            }
             if (user.getUserType() != UserType.ADMIN_USER){
                 List<T> unauthorizedList = processService.listUnauthorized(userId,needChecks,authorizationType);
                 // if exist unauthorized resource
diff --git a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
index 0bab35a821..aa7d83f4a8 100644
--- a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
+++ b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
@@ -1813,7 +1813,7 @@ public class ProcessService {
      * @return User
      */
     public User getUserById(int userId){
-        return userMapper.queryDetailsById(userId);
+        return userMapper.selectById(userId);
     }
 
     /**