feat:optimizing code (#2245)

* feat:optimizing code

* add licence and  format code

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertGroupController.java

@@ -93,11 +93,11 @@ public class AlertGroupController extends  BaseController{
     public Result list(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {
         logger.info("login  user {}, query all alertGroup",
                 loginUser.getUserName());
-        try{
+        try {
             HashMap<String, Object> result = alertGroupService.queryAlertgroup();
             return returnDataList(result);
-        }catch (Exception e){
+        } catch (Exception e) {
-            logger.error(Status.QUERY_ALL_ALERTGROUP_ERROR.getMsg(),e);
+            logger.error(Status.QUERY_ALL_ALERTGROUP_ERROR.getMsg(), e);
             return error(Status.QUERY_ALL_ALERTGROUP_ERROR.getCode(), Status.QUERY_ALL_ALERTGROUP_ERROR.getMsg());
         }
     }
@@ -214,12 +214,20 @@ public class AlertGroupController extends  BaseController{
     @GetMapping(value = "/verify-group-name")
     @ResponseStatus(HttpStatus.OK)
     public Result verifyGroupName(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
-                                 @RequestParam(value ="groupName") String groupName
+                                 @RequestParam(value ="groupName") String groupName) {
-    ) {
+        logger.info("login user {}, verify group name: {}", loginUser.getUserName(), groupName);
-        logger.info("login user {}, verfiy group name: {}",
-                loginUser.getUserName(),groupName);
 
-        return alertGroupService.verifyGroupName(loginUser, groupName);
+        boolean exist= alertGroupService.existGroupName(groupName);
+        Result result = new Result();
+        if (exist) {
+            logger.error("group {} has exist, can't create again.", groupName);
+            result.setCode(Status.ALERT_GROUP_EXIST.getCode());
+            result.setMsg(Status.ALERT_GROUP_EXIST.getMsg());
+        } else {
+            result.setCode(Status.SUCCESS.getCode());
+            result.setMsg(Status.SUCCESS.getMsg());
+        }
+        return result;
     }
 
     /**

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertGroupService.java

@@ -16,17 +16,17 @@
  */
 package org.apache.dolphinscheduler.api.service;
 
+import java.util.*;
 import org.apache.dolphinscheduler.api.enums.Status;
 import org.apache.dolphinscheduler.api.utils.PageInfo;
-import org.apache.dolphinscheduler.api.utils.Result;
 import org.apache.dolphinscheduler.common.Constants;
 import org.apache.dolphinscheduler.common.enums.AlertType;
+import org.apache.dolphinscheduler.common.utils.CollectionUtils;
 import org.apache.dolphinscheduler.common.utils.StringUtils;
 import org.apache.dolphinscheduler.dao.entity.AlertGroup;
 import org.apache.dolphinscheduler.dao.entity.User;
 import org.apache.dolphinscheduler.dao.entity.UserAlertGroup;
 import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper;
-import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper;
 import com.baomidou.mybatisplus.core.metadata.IPage;
 import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
 import org.slf4j.Logger;
@@ -35,11 +35,6 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
 /**
  * alert group service
  */
@@ -52,8 +47,7 @@ public class AlertGroupService extends BaseService{
     private AlertGroupMapper alertGroupMapper;
 
     @Autowired
-    private UserAlertGroupMapper userAlertGroupMapper;
+    private UserAlertGroupService userAlertGroupService;
-
     /**
      * query alert group list
      *
@@ -122,7 +116,7 @@ public class AlertGroupService extends BaseService{
         alertGroup.setCreateTime(now);
         alertGroup.setUpdateTime(now);
 
-        // insert 
+        // insert
         int insert = alertGroupMapper.insert(alertGroup);
 
         if (insert > 0) {
@@ -199,7 +193,7 @@ public class AlertGroupService extends BaseService{
             return result;
         }
 
-        userAlertGroupMapper.deleteByAlertgroupId(id);
+        userAlertGroupService.deleteByAlertGroupId(id);
         alertGroupMapper.deleteById(id);
         putMsg(result, Status.SUCCESS);
         return result;
@@ -223,22 +217,26 @@ public class AlertGroupService extends BaseService{
             return result;
         }
 
-        userAlertGroupMapper.deleteByAlertgroupId(alertgroupId);
+        userAlertGroupService.deleteByAlertGroupId(alertgroupId);
         if (StringUtils.isEmpty(userIds)) {
             putMsg(result, Status.SUCCESS);
             return result;
         }
 
         String[] userIdsArr = userIds.split(",");
-
+        Date now = new Date();
+        List<UserAlertGroup> alertGroups = new ArrayList<>(userIds.length());
         for (String userId : userIdsArr) {
-            Date now = new Date();
             UserAlertGroup userAlertGroup = new UserAlertGroup();
             userAlertGroup.setAlertgroupId(alertgroupId);
             userAlertGroup.setUserId(Integer.parseInt(userId));
             userAlertGroup.setCreateTime(now);
             userAlertGroup.setUpdateTime(now);
-            userAlertGroupMapper.insert(userAlertGroup);
+            alertGroups.add(userAlertGroup);
+        }
+
+        if (CollectionUtils.isNotEmpty(alertGroups)) {
+            userAlertGroupService.saveBatch(alertGroups);
         }
 
         putMsg(result, Status.SUCCESS);
@@ -248,22 +246,11 @@ public class AlertGroupService extends BaseService{
     /**
      * verify group name exists
      *
-     * @param loginUser login user
      * @param groupName group name
      * @return check result code
      */
-    public Result verifyGroupName(User loginUser, String groupName) {
+    public boolean existGroupName(String groupName) {
-        Result result = new Result();
         List<AlertGroup> alertGroup = alertGroupMapper.queryByGroupName(groupName);
-        if (alertGroup != null && alertGroup.size() > 0) {
+        return CollectionUtils.isNotEmpty(alertGroup);
-            logger.error("group {} has exist, can't create again.", groupName);
-            result.setCode(Status.ALERT_GROUP_EXIST.getCode());
-            result.setMsg(Status.ALERT_GROUP_EXIST.getMsg());
-        } else {
-            result.setCode(Status.SUCCESS.getCode());
-            result.setMsg(Status.SUCCESS.getMsg());
-        }
-
-        return result;
     }
 }

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UserAlertGroupService.java

@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dolphinscheduler.api.service;
+
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import org.apache.dolphinscheduler.dao.entity.UserAlertGroup;
+import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+
+/**
+ *
+ */
+@Service
+public class UserAlertGroupService extends ServiceImpl<UserAlertGroupMapper, UserAlertGroup> {
+
+    @Autowired
+    private UserAlertGroupMapper userAlertGroupMapper;
+
+    boolean deleteByAlertGroupId(Integer groupId) {
+        return userAlertGroupMapper.deleteByAlertgroupId(groupId) >= 1;
+    }
+
+}

dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertGroupServiceTest.java

@@ -18,9 +18,12 @@ package org.apache.dolphinscheduler.api.service;
 
 import com.baomidou.mybatisplus.core.metadata.IPage;
 import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import org.apache.dolphinscheduler.api.enums.Status;
 import org.apache.dolphinscheduler.api.utils.PageInfo;
-import org.apache.dolphinscheduler.api.utils.Result;
 import org.apache.dolphinscheduler.common.Constants;
 import org.apache.dolphinscheduler.common.enums.AlertType;
 import org.apache.dolphinscheduler.common.enums.UserType;
@@ -31,9 +34,12 @@ import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper;
 import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper;
 import org.junit.After;
 import org.junit.Assert;
+import static org.junit.Assert.assertEquals;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import static org.mockito.ArgumentMatchers.*;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
@@ -41,14 +47,6 @@ import org.mockito.junit.MockitoJUnitRunner;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
-
 @RunWith(MockitoJUnitRunner.class)
 public class AlertGroupServiceTest {
 
@@ -60,6 +58,8 @@ public class AlertGroupServiceTest {
     private AlertGroupMapper alertGroupMapper;
     @Mock
     private UserAlertGroupMapper userAlertGroupMapper;
+    @Mock
+    UserAlertGroupService userAlertGroupService;
 
     private String groupName = "AlertGroupServiceTest";
 
@@ -160,25 +160,34 @@ public class AlertGroupServiceTest {
 
 
     }
+
     @Test
-    public  void testGrantUser(){
+    public void testGrantUser() {
+
+        Integer groupId = 1;
+
+        ArgumentCaptor<Integer> groupArgument = ArgumentCaptor.forClass(Integer.class);
+
+        Mockito.when(userAlertGroupService.deleteByAlertGroupId(anyInt())).thenReturn(true);
+
+        Map<String, Object> result = alertGroupService.grantUser(getLoginUser(), groupId, "123,321");
+        Mockito.verify(userAlertGroupService).deleteByAlertGroupId(groupArgument.capture());
 
-        Map<String, Object>  result = alertGroupService.grantUser(getLoginUser(),1,"123,321");
         logger.info(result.toString());
-        Assert.assertEquals(Status.SUCCESS,result.get(Constants.STATUS));
+        assertEquals(groupArgument.getValue(), groupId);
+        assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
     }
+
     @Test
-    public  void testVerifyGroupName(){
+    public void testVerifyGroupName() {
         //group name not exist
-        Result result = alertGroupService.verifyGroupName(getLoginUser(), groupName);
+        boolean result = alertGroupService.existGroupName(groupName);
-        logger.info(result.toString());
+        Assert.assertFalse(result);
-        Assert.assertEquals(Status.SUCCESS.getMsg(),result.getMsg());
         Mockito.when(alertGroupMapper.queryByGroupName(groupName)).thenReturn(getList());
 
         //group name exist
-        result = alertGroupService.verifyGroupName(getLoginUser(), groupName);
+        result = alertGroupService.existGroupName(groupName);
-        logger.info(result.toString());
+        Assert.assertTrue(result);
-        Assert.assertEquals(Status.ALERT_GROUP_EXIST.getMsg(),result.getMsg());
     }
 
 

dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UserAlertGroupServiceTest.java

@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dolphinscheduler.api.service;
+
+import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper;
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+/**
+ *
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class UserAlertGroupServiceTest {
+
+    @InjectMocks
+    UserAlertGroupService userAlertGroupService;
+
+    @Mock
+    UserAlertGroupMapper userAlertGroupMapper;
+
+    @Test
+    public void deleteByAlertGroupId() {
+
+        Integer groupId = 1;
+        userAlertGroupService.deleteByAlertGroupId(groupId);
+        ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
+
+        Mockito.verify(userAlertGroupMapper).deleteByAlertgroupId(argumentCaptor.capture());
+        assertEquals(argumentCaptor.getValue(), groupId);
+
+    }
+
+}

pom.xml

@@ -704,6 +704,7 @@
                         <include>**/api/service/TenantServiceTest.java</include>
                         <include>**/api/service/WorkerGroupServiceTest.java</include>
                         <include>**/api/service/AlertGroupServiceTest.java</include>
+                        <include>**/api/service/UserAlertGroupServiceTest.java</include>
                         <include>**/api/service/ProjectServiceTest.java</include>
                         <include>**/api/service/ProcessDefinitionServiceTest.java</include>
                         <include>**/api/service/UdfFuncServiceTest.java</include>