From 3a8b80971ba4b23b8e919ad04e074bd273c40c9a Mon Sep 17 00:00:00 2001 From: BenjaminWenqiYu <41983209+BenjaminWenqiYu@users.noreply.github.com> Date: Sun, 31 Oct 2021 21:26:48 +0800 Subject: [PATCH] 1. ResourceMapper rename the original 'existResource' function to 'existResourceByUser' (#6409) 2. ResourceMapper add existResource function without the userId condition 3. change the ResourceServiceImpl's private function 'checkResourceExists' without userId condition --- .../api/service/impl/ResourcesServiceImpl.java | 13 ++++++------- .../api/service/ResourcesServiceTest.java | 6 +++--- .../dolphinscheduler/dao/mapper/ResourceMapper.java | 12 +++++++++++- .../dolphinscheduler/dao/mapper/ResourceMapper.xml | 9 ++++++++- .../dao/mapper/ResourceMapperTest.java | 6 ++++-- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java index 7835716551..5d1ba68593 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java @@ -136,7 +136,7 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe return result; } - if (checkResourceExists(fullName, 0, type.ordinal())) { + if (checkResourceExists(fullName, type.ordinal())) { logger.error("resource directory {} has exist, can't recreate", fullName); putMsg(result, Status.RESOURCE_EXIST); return result; @@ -208,7 +208,7 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe // check resource name exists String fullName = currentDir.equals("/") ? String.format("%s%s",currentDir,name) : String.format("%s/%s",currentDir,name); - if (checkResourceExists(fullName, 0, type.ordinal())) { + if (checkResourceExists(fullName, type.ordinal())) { logger.error("resource {} has exist, can't recreate", RegexUtils.escapeNRT(name)); putMsg(result, Status.RESOURCE_EXIST); return result; @@ -246,12 +246,11 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe * check resource is exists * * @param fullName fullName - * @param userId user id * @param type type * @return true if resource exists */ - private boolean checkResourceExists(String fullName, int userId, int type) { - Boolean existResource = resourcesMapper.existResource(fullName, userId, type); + private boolean checkResourceExists(String fullName, int type) { + Boolean existResource = resourcesMapper.existResource(fullName, type); return existResource == Boolean.TRUE; } @@ -298,7 +297,7 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe String originResourceName = resource.getAlias(); String fullName = String.format("%s%s",originFullName.substring(0,originFullName.lastIndexOf("/") + 1),name); - if (!originResourceName.equals(name) && checkResourceExists(fullName, 0, type.ordinal())) { + if (!originResourceName.equals(name) && checkResourceExists(fullName, type.ordinal())) { logger.error("resource {} already exists, can't recreate", name); putMsg(result, Status.RESOURCE_EXIST); return result; @@ -751,7 +750,7 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe public Result verifyResourceName(String fullName, ResourceType type, User loginUser) { Result result = new Result<>(); putMsg(result, Status.SUCCESS); - if (checkResourceExists(fullName, 0, type.ordinal())) { + if (checkResourceExists(fullName, type.ordinal())) { logger.error("resource type:{} name:{} has exist, can't create again.", type, RegexUtils.escapeNRT(fullName)); putMsg(result, Status.RESOURCE_EXIST); } else { 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 cca867c8ac..6afbf7b965 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 @@ -169,7 +169,7 @@ public class ResourcesServiceTest { Assert.assertEquals(Status.PARENT_RESOURCE_NOT_EXIST.getMsg(), result.getMsg()); //RESOURCE_EXIST PowerMockito.when(PropertyUtils.getResUploadStartupState()).thenReturn(true); - Mockito.when(resourcesMapper.existResource("/directoryTest", 0, 0)).thenReturn(true); + Mockito.when(resourcesMapper.existResource("/directoryTest", 0)).thenReturn(true); result = resourcesService.createDirectory(user, "directoryTest", "directory test", ResourceType.FILE, -1, "/"); logger.info(result.toString()); Assert.assertEquals(Status.RESOURCE_EXIST.getMsg(), result.getMsg()); @@ -227,7 +227,7 @@ public class ResourcesServiceTest { Assert.assertEquals(Status.SUCCESS.getMsg(), result.getMsg()); //RESOURCE_EXIST - Mockito.when(resourcesMapper.existResource("/ResourcesServiceTest1.jar", 0, 0)).thenReturn(true); + Mockito.when(resourcesMapper.existResource("/ResourcesServiceTest1.jar", 0)).thenReturn(true); result = resourcesService.updateResource(user, 1, "ResourcesServiceTest1.jar", "ResourcesServiceTest", ResourceType.FILE, null); logger.info(result.toString()); Assert.assertEquals(Status.RESOURCE_EXIST.getMsg(), result.getMsg()); @@ -345,7 +345,7 @@ public class ResourcesServiceTest { User user = new User(); user.setId(1); - Mockito.when(resourcesMapper.existResource("/ResourcesServiceTest.jar", 0, 0)).thenReturn(true); + Mockito.when(resourcesMapper.existResource("/ResourcesServiceTest.jar", 0)).thenReturn(true); Result result = resourcesService.verifyResourceName("/ResourcesServiceTest.jar", ResourceType.FILE, user); logger.info(result.toString()); Assert.assertEquals(Status.RESOURCE_EXIST.getMsg(), result.getMsg()); diff --git a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java index 8fbd9a34b9..7b3c1c905a 100644 --- a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java +++ b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java @@ -143,7 +143,17 @@ public interface ResourceMapper extends BaseMapper { * @param type type * @return true if exist else return null */ - Boolean existResource(@Param("fullName") String fullName, + Boolean existResourceByUser(@Param("fullName") String fullName, @Param("userId") int userId, @Param("type") int type); + + /** + * check resource exist + * @param fullName full name + * @param type type + * @return true if exist else return null + */ + Boolean existResource(@Param("fullName") String fullName, + @Param("type") int type); + } diff --git a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml index 7a8b043c09..45f956521d 100644 --- a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml +++ b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml @@ -178,11 +178,18 @@ - select 1 from t_ds_resources where full_name = #{fullName} and type = #{type} and user_id = #{userId} limit 1 + + diff --git a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapperTest.java b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapperTest.java index e2ba437c3a..0777ba52be 100644 --- a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapperTest.java +++ b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapperTest.java @@ -426,9 +426,11 @@ public class ResourceMapperTest { String fullName = "/ut-resource"; int userId = 111; int type = ResourceType.FILE.getCode(); - Assert.assertNull(resourceMapper.existResource(fullName, userId, type)); + Assert.assertNull(resourceMapper.existResourceByUser(fullName, userId, type)); + Assert.assertNull(resourceMapper.existResource(fullName, type)); insertOne(); - Assert.assertTrue(resourceMapper.existResource(fullName, userId, type)); + Assert.assertTrue(resourceMapper.existResourceByUser(fullName, userId, type)); + Assert.assertTrue(resourceMapper.existResource(fullName, type)); } }