You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by le...@apache.org on 2020/06/03 05:50:17 UTC
[incubator-dolphinscheduler] branch dev-1.3.0 updated: Fix bug:If
user didn't set the value of tenant, release process definition fail (#2885)
This is an automated email from the ASF dual-hosted git repository.
leonbao pushed a commit to branch dev-1.3.0
in repository https://gitbox.apache.org/repos/asf/incubator-dolphinscheduler.git
The following commit(s) were added to refs/heads/dev-1.3.0 by this push:
new 4441d91 Fix bug:If user didn't set the value of tenant,release process definition fail (#2885)
4441d91 is described below
commit 4441d91dcf178653e55b94fe564c8c9907e897fa
Author: lgcareer <18...@163.com>
AuthorDate: Wed Jun 3 13:50:06 2020 +0800
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 +++++++-------
.../dolphinscheduler/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 18ffa50..173856b 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 e52f467..407f6b5 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 fcf8a13..003a412 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 9f93f4c..1a9295b 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 0bab35a..aa7d83f 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);
}
/**