[Improvement-3369][api] Introduce alert group and users service interface for clear code (#4758)

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

@@ -32,7 +32,6 @@ import org.apache.dolphinscheduler.common.utils.ParameterUtils;
 import org.apache.dolphinscheduler.common.utils.StringUtils;
 import org.apache.dolphinscheduler.dao.entity.User;
 
-import java.util.HashMap;
 import java.util.Map;
 
 import org.slf4j.Logger;
@@ -111,7 +110,7 @@ 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());
-        HashMap<String, Object> result = alertGroupService.queryAlertgroup();
+        Map<String, Object> result = alertGroupService.queryAlertgroup();
         return returnDataList(result);
     }
 

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

@@ -17,55 +17,21 @@
 
 package org.apache.dolphinscheduler.api.service;
 
-import org.apache.dolphinscheduler.api.enums.Status;
-import org.apache.dolphinscheduler.api.utils.PageInfo;
-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.mapper.AlertGroupMapper;
 
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Service;
-import org.springframework.transaction.annotation.Transactional;
-
-import com.baomidou.mybatisplus.core.metadata.IPage;
-import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
-
 /**
  * alert group service
  */
-@Service
+public interface AlertGroupService {
-public class AlertGroupService extends BaseService {
-
-    private static final Logger logger = LoggerFactory.getLogger(AlertGroupService.class);
-
-    @Autowired
-    private AlertGroupMapper alertGroupMapper;
 
     /**
      * query alert group list
      *
      * @return alert group list
      */
-    public HashMap<String, Object> queryAlertgroup() {
+    Map<String, Object> queryAlertgroup();
-
-        HashMap<String, Object> result = new HashMap<>();
-        List<AlertGroup> alertGroups = alertGroupMapper.queryAllGroupList();
-        result.put(Constants.DATA_LIST, alertGroups);
-        putMsg(result, Status.SUCCESS);
-
-        return result;
-    }
 
     /**
      * paging query alarm group list
@@ -76,24 +42,7 @@ public class AlertGroupService extends BaseService {
      * @param pageSize page size
      * @return alert group list page
      */
-    public Map<String, Object> listPaging(User loginUser, String searchVal, Integer pageNo, Integer pageSize) {
+    Map<String, Object> listPaging(User loginUser, String searchVal, Integer pageNo, Integer pageSize);
-
-        Map<String, Object> result = new HashMap<>();
-        if (isNotAdmin(loginUser, result)) {
-            return result;
-        }
-
-        Page<AlertGroup> page = new Page(pageNo, pageSize);
-        IPage<AlertGroup> alertGroupIPage = alertGroupMapper.queryAlertGroupPage(
-            page, searchVal);
-        PageInfo<AlertGroup> pageInfo = new PageInfo<>(pageNo, pageSize);
-        pageInfo.setTotalCount((int) alertGroupIPage.getTotal());
-        pageInfo.setLists(alertGroupIPage.getRecords());
-        result.put(Constants.DATA_LIST, pageInfo);
-        putMsg(result, Status.SUCCESS);
-
-        return result;
-    }
 
     /**
      * create alert group
@@ -104,33 +53,7 @@ public class AlertGroupService extends BaseService {
      * @param alertInstanceIds alertInstanceIds
      * @return create result code
      */
-    public Map<String, Object> createAlertgroup(User loginUser, String groupName, String desc, String alertInstanceIds) {
+    Map<String, Object> createAlertgroup(User loginUser, String groupName, String desc, String alertInstanceIds);
-        Map<String, Object> result = new HashMap<>();
-        //only admin can operate
-        if (isNotAdmin(loginUser, result)) {
-            return result;
-        }
-
-        AlertGroup alertGroup = new AlertGroup();
-        Date now = new Date();
-
-        alertGroup.setGroupName(groupName);
-        alertGroup.setAlertInstanceIds(alertInstanceIds);
-        alertGroup.setDescription(desc);
-        alertGroup.setCreateTime(now);
-        alertGroup.setUpdateTime(now);
-        alertGroup.setCreateUserId(loginUser.getId());
-
-        // insert
-        int insert = alertGroupMapper.insert(alertGroup);
-
-        if (insert > 0) {
-            putMsg(result, Status.SUCCESS);
-        } else {
-            putMsg(result, Status.CREATE_ALERT_GROUP_ERROR);
-        }
-        return result;
-    }
 
     /**
      * updateProcessInstance alert group
@@ -142,35 +65,7 @@ public class AlertGroupService extends BaseService {
      * @param alertInstanceIds alertInstanceIds
      * @return update result code
      */
-    public Map<String, Object> updateAlertgroup(User loginUser, int id, String groupName, String desc, String alertInstanceIds) {
+    Map<String, Object> updateAlertgroup(User loginUser, int id, String groupName, String desc, String alertInstanceIds);
-        Map<String, Object> result = new HashMap<>();
-
-        if (isNotAdmin(loginUser, result)) {
-            return result;
-        }
-
-        AlertGroup alertGroup = alertGroupMapper.selectById(id);
-
-        if (alertGroup == null) {
-            putMsg(result, Status.ALERT_GROUP_NOT_EXIST);
-            return result;
-
-        }
-
-        Date now = new Date();
-
-        if (StringUtils.isNotEmpty(groupName)) {
-            alertGroup.setGroupName(groupName);
-        }
-        alertGroup.setDescription(desc);
-        alertGroup.setUpdateTime(now);
-        alertGroup.setCreateUserId(loginUser.getId());
-        alertGroup.setAlertInstanceIds(alertInstanceIds);
-        // updateProcessInstance
-        alertGroupMapper.updateById(alertGroup);
-        putMsg(result, Status.SUCCESS);
-        return result;
-    }
 
     /**
      * delete alert group by id
@@ -179,25 +74,7 @@ public class AlertGroupService extends BaseService {
      * @param id alert group id
      * @return delete result code
      */
-    @Transactional(rollbackFor = RuntimeException.class)
+    Map<String, Object> delAlertgroupById(User loginUser, int id);
-    public Map<String, Object> delAlertgroupById(User loginUser, int id) {
-        Map<String, Object> result = new HashMap<>();
-        result.put(Constants.STATUS, false);
-
-        //only admin can operate
-        if (isNotAdmin(loginUser, result)) {
-            return result;
-        }
-        //check exist
-        AlertGroup alertGroup = alertGroupMapper.selectById(id);
-        if (alertGroup == null) {
-            putMsg(result, Status.ALERT_GROUP_NOT_EXIST);
-            return result;
-        }
-        alertGroupMapper.deleteById(id);
-        putMsg(result, Status.SUCCESS);
-        return result;
-    }
 
     /**
      * verify group name exists
@@ -205,8 +82,5 @@ public class AlertGroupService extends BaseService {
      * @param groupName group name
      * @return check result code
      */
-    public boolean existGroupName(String groupName) {
+    boolean existGroupName(String groupName);
-        List<AlertGroup> alertGroup = alertGroupMapper.queryByGroupName(groupName);
-        return CollectionUtils.isNotEmpty(alertGroup);
-    }
 }

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

@@ -16,20 +16,17 @@
  */
 package org.apache.dolphinscheduler.api.service;
 
-import java.text.MessageFormat;
-import java.util.Map;
-
-import javax.servlet.http.Cookie;
-import javax.servlet.http.HttpServletRequest;
-
 import org.apache.dolphinscheduler.api.enums.Status;
 import org.apache.dolphinscheduler.api.utils.Result;
 import org.apache.dolphinscheduler.common.Constants;
 import org.apache.dolphinscheduler.common.enums.UserType;
 import org.apache.dolphinscheduler.common.utils.HadoopUtils;
-import org.apache.dolphinscheduler.common.utils.StringUtils;
 import org.apache.dolphinscheduler.dao.entity.User;
 
+import java.io.IOException;
+import java.text.MessageFormat;
+import java.util.Map;
+
 /**
  * base service
  */
@@ -117,9 +114,9 @@ public class BaseService {
      * create tenant dir if not exists
      *
      * @param tenantCode tenant code
-     * @throws Exception if hdfs operation exception
+     * @throws IOException if hdfs operation exception
      */
-    protected void createTenantDirIfNotExists(String tenantCode) throws Exception {
+    protected void createTenantDirIfNotExists(String tenantCode) throws IOException {
 
         String resourcePath = HadoopUtils.getHdfsResDir(tenantCode);
         String udfsPath = HadoopUtils.getHdfsUdfDir(tenantCode);

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertGroupServiceImpl.java

@@ -0,0 +1,208 @@
+/*
+ * 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.impl;
+
+import org.apache.dolphinscheduler.api.enums.Status;
+import org.apache.dolphinscheduler.api.service.AlertGroupService;
+import org.apache.dolphinscheduler.api.service.BaseService;
+import org.apache.dolphinscheduler.api.utils.PageInfo;
+import org.apache.dolphinscheduler.common.Constants;
+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.mapper.AlertGroupMapper;
+
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+
+/**
+ * alert group service impl
+ */
+@Service
+public class AlertGroupServiceImpl extends BaseService implements AlertGroupService {
+
+    @Autowired
+    private AlertGroupMapper alertGroupMapper;
+
+    /**
+     * query alert group list
+     *
+     * @return alert group list
+     */
+    public Map<String, Object> queryAlertgroup() {
+
+        HashMap<String, Object> result = new HashMap<>();
+        List<AlertGroup> alertGroups = alertGroupMapper.queryAllGroupList();
+        result.put(Constants.DATA_LIST, alertGroups);
+        putMsg(result, Status.SUCCESS);
+
+        return result;
+    }
+
+    /**
+     * paging query alarm group list
+     *
+     * @param loginUser login user
+     * @param searchVal search value
+     * @param pageNo page number
+     * @param pageSize page size
+     * @return alert group list page
+     */
+    public Map<String, Object> listPaging(User loginUser, String searchVal, Integer pageNo, Integer pageSize) {
+
+        Map<String, Object> result = new HashMap<>();
+        if (isNotAdmin(loginUser, result)) {
+            return result;
+        }
+
+        Page<AlertGroup> page = new Page<>(pageNo, pageSize);
+        IPage<AlertGroup> alertGroupIPage = alertGroupMapper.queryAlertGroupPage(
+                page, searchVal);
+        PageInfo<AlertGroup> pageInfo = new PageInfo<>(pageNo, pageSize);
+        pageInfo.setTotalCount((int) alertGroupIPage.getTotal());
+        pageInfo.setLists(alertGroupIPage.getRecords());
+        result.put(Constants.DATA_LIST, pageInfo);
+        putMsg(result, Status.SUCCESS);
+
+        return result;
+    }
+
+    /**
+     * create alert group
+     *
+     * @param loginUser login user
+     * @param groupName group name
+     * @param desc description
+     * @param alertInstanceIds alertInstanceIds
+     * @return create result code
+     */
+    public Map<String, Object> createAlertgroup(User loginUser, String groupName, String desc, String alertInstanceIds) {
+        Map<String, Object> result = new HashMap<>();
+        //only admin can operate
+        if (isNotAdmin(loginUser, result)) {
+            return result;
+        }
+
+        AlertGroup alertGroup = new AlertGroup();
+        Date now = new Date();
+
+        alertGroup.setGroupName(groupName);
+        alertGroup.setAlertInstanceIds(alertInstanceIds);
+        alertGroup.setDescription(desc);
+        alertGroup.setCreateTime(now);
+        alertGroup.setUpdateTime(now);
+        alertGroup.setCreateUserId(loginUser.getId());
+
+        // insert
+        int insert = alertGroupMapper.insert(alertGroup);
+
+        if (insert > 0) {
+            putMsg(result, Status.SUCCESS);
+        } else {
+            putMsg(result, Status.CREATE_ALERT_GROUP_ERROR);
+        }
+        return result;
+    }
+
+    /**
+     * updateProcessInstance alert group
+     *
+     * @param loginUser login user
+     * @param id alert group id
+     * @param groupName group name
+     * @param desc description
+     * @param alertInstanceIds alertInstanceIds
+     * @return update result code
+     */
+    public Map<String, Object> updateAlertgroup(User loginUser, int id, String groupName, String desc, String alertInstanceIds) {
+        Map<String, Object> result = new HashMap<>();
+
+        if (isNotAdmin(loginUser, result)) {
+            return result;
+        }
+
+        AlertGroup alertGroup = alertGroupMapper.selectById(id);
+
+        if (alertGroup == null) {
+            putMsg(result, Status.ALERT_GROUP_NOT_EXIST);
+            return result;
+
+        }
+
+        Date now = new Date();
+
+        if (StringUtils.isNotEmpty(groupName)) {
+            alertGroup.setGroupName(groupName);
+        }
+        alertGroup.setDescription(desc);
+        alertGroup.setUpdateTime(now);
+        alertGroup.setCreateUserId(loginUser.getId());
+        alertGroup.setAlertInstanceIds(alertInstanceIds);
+        alertGroupMapper.updateById(alertGroup);
+        putMsg(result, Status.SUCCESS);
+        return result;
+    }
+
+    /**
+     * delete alert group by id
+     *
+     * @param loginUser login user
+     * @param id alert group id
+     * @return delete result code
+     */
+    @Transactional(rollbackFor = RuntimeException.class)
+    public Map<String, Object> delAlertgroupById(User loginUser, int id) {
+        Map<String, Object> result = new HashMap<>();
+        result.put(Constants.STATUS, false);
+
+        //only admin can operate
+        if (isNotAdmin(loginUser, result)) {
+            return result;
+        }
+        //check exist
+        AlertGroup alertGroup = alertGroupMapper.selectById(id);
+        if (alertGroup == null) {
+            putMsg(result, Status.ALERT_GROUP_NOT_EXIST);
+            return result;
+        }
+        alertGroupMapper.deleteById(id);
+        putMsg(result, Status.SUCCESS);
+        return result;
+    }
+
+    /**
+     * verify group name exists
+     *
+     * @param groupName group name
+     * @return check result code
+     */
+    public boolean existGroupName(String groupName) {
+        List<AlertGroup> alertGroup = alertGroupMapper.queryByGroupName(groupName);
+        return CollectionUtils.isNotEmpty(alertGroup);
+    }
+}

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

@@ -21,9 +21,9 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 
 import org.apache.dolphinscheduler.api.enums.Status;
+import org.apache.dolphinscheduler.api.service.impl.AlertGroupServiceImpl;
 import org.apache.dolphinscheduler.api.utils.PageInfo;
 import org.apache.dolphinscheduler.common.Constants;
-import org.apache.dolphinscheduler.common.enums.AlertType;
 import org.apache.dolphinscheduler.common.enums.UserType;
 import org.apache.dolphinscheduler.common.utils.CollectionUtils;
 import org.apache.dolphinscheduler.dao.entity.AlertGroup;
@@ -31,7 +31,6 @@ import org.apache.dolphinscheduler.dao.entity.User;
 import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -54,7 +53,7 @@ public class AlertGroupServiceTest {
     private static final Logger logger = LoggerFactory.getLogger(AlertGroupServiceTest.class);
 
     @InjectMocks
-    private AlertGroupService alertGroupService;
+    private AlertGroupServiceImpl alertGroupService;
     @Mock
     private AlertGroupMapper alertGroupMapper;
 
@@ -64,7 +63,7 @@ public class AlertGroupServiceTest {
     public void testQueryAlertGroup() {
 
         Mockito.when(alertGroupMapper.queryAllGroupList()).thenReturn(getList());
-        HashMap<String, Object> result = alertGroupService.queryAlertgroup();
+        Map<String, Object> result = alertGroupService.queryAlertgroup();
         logger.info(result.toString());
         List<AlertGroup> alertGroups = (List<AlertGroup>) result.get(Constants.DATA_LIST);
         Assert.assertTrue(CollectionUtils.isNotEmpty(alertGroups));

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

@@ -22,6 +22,7 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
 
 import org.apache.dolphinscheduler.api.enums.Status;
+import org.apache.dolphinscheduler.api.service.impl.UsersServiceImpl;
 import org.apache.dolphinscheduler.api.utils.PageInfo;
 import org.apache.dolphinscheduler.api.utils.Result;
 import org.apache.dolphinscheduler.common.Constants;
@@ -65,7 +66,7 @@ public class UsersServiceTest {
     private static final Logger logger = LoggerFactory.getLogger(UsersServiceTest.class);
 
     @InjectMocks
-    private UsersService usersService;
+    private UsersServiceImpl usersService;
     @Mock
     private UserMapper userMapper;
     @Mock

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/PropertyUtils.java

@@ -73,7 +73,7 @@ public class PropertyUtils {
     /**
      * @return judge whether resource upload startup
      */
-    public static Boolean getResUploadStartupState() {
+    public static boolean getResUploadStartupState() {
         String resUploadStartupType = PropertyUtils.getUpperCaseString(Constants.RESOURCE_STORAGE_TYPE);
         ResUploadType resUploadType = ResUploadType.valueOf(resUploadStartupType);
         return resUploadType == ResUploadType.HDFS || resUploadType == ResUploadType.S3;