Merge pull request #4572 from CalvinKirs/fix_alert_error

[Improvement][Alert]Alert problem repair and optimization

dolphinscheduler-alert/src/main/java/org/apache/dolphinscheduler/alert/runner/AlertSender.java

@@ -75,13 +75,14 @@ public class AlertSender {
             List<AlertPluginInstance> alertInstanceList = alertDao.listInstanceByAlertGroupId(alertGroupId);
             if (CollectionUtils.isEmpty(alertInstanceList)) {
                 logger.error("send alert msg fail,no bind plugin instance.");
-                return;
+                alertDao.updateAlert(AlertStatus.EXECUTION_FAILURE, "no bind plugin instance", alert.getId());
+                continue;
             }
             AlertData alertData = new AlertData();
             alertData.setId(alert.getId())
-                .setContent(alert.getContent())
-                .setLog(alert.getLog())
-                .setTitle(alert.getTitle());
+                    .setContent(alert.getContent())
+                    .setLog(alert.getLog())
+                    .setTitle(alert.getTitle());
 
             for (AlertPluginInstance instance : alertInstanceList) {
 
@@ -126,7 +127,7 @@ public class AlertSender {
         for (AlertPluginInstance instance : alertInstanceList) {
             AlertResult alertResult = this.alertResultHandler(instance, alertData);
             AlertSendResponseResult alertSendResponseResult = new AlertSendResponseResult(
-                Boolean.parseBoolean(String.valueOf(alertResult.getStatus())), alertResult.getMessage());
+                    Boolean.parseBoolean(String.valueOf(alertResult.getStatus())), alertResult.getMessage());
             sendResponseStatus = sendResponseStatus && alertSendResponseResult.getStatus();
             sendResponseResults.add(alertSendResponseResult);
         }

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/enums/Status.java

@@ -290,6 +290,7 @@ public enum Status {
     QUERY_ALL_ALERT_PLUGIN_INSTANCE_ERROR(110009, "query all alert plugin instance error", "查询所有告警实例失败"),
     PLUGIN_INSTANCE_ALREADY_EXIT(110010,"plugin instance already exit","该告警插件实例已存在"),
     LIST_PAGING_ALERT_PLUGIN_INSTANCE_ERROR(110011,"query plugin instance page error","分页查询告警实例失败"),
+    DELETE_ALERT_PLUGIN_INSTANCE_ERROR_HAS_ALERT_GROUP_ASSOCIATED(110012,"failed to delete the alert instance, there is an alarm group associated with this alert instance","删除告警实例失败，存在与此告警实例关联的警报组")
 
     ;
 

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

@@ -27,13 +27,16 @@ import org.apache.dolphinscheduler.common.utils.CollectionUtils;
 import org.apache.dolphinscheduler.dao.entity.AlertPluginInstance;
 import org.apache.dolphinscheduler.dao.entity.PluginDefine;
 import org.apache.dolphinscheduler.dao.entity.User;
+import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper;
 import org.apache.dolphinscheduler.dao.mapper.AlertPluginInstanceMapper;
 import org.apache.dolphinscheduler.dao.mapper.PluginDefineMapper;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 import org.springframework.beans.factory.annotation.Autowired;
@@ -56,6 +59,9 @@ public class AlertPluginInstanceServiceImpl extends BaseService implements Alert
     @Autowired
     private PluginDefineMapper pluginDefineMapper;
 
+    @Autowired
+    private AlertGroupMapper alertGroupMapper;
+
     /**
      * creat alert plugin instance
      *
@@ -121,6 +127,13 @@ public class AlertPluginInstanceServiceImpl extends BaseService implements Alert
     @Override
     public Map<String, Object> delete(User loginUser, int id) {
         Map<String, Object> result = new HashMap<>();
+        //check if there is an associated alert group
+        boolean hasAssociatedAlertGroup = checkHasAssociatedAlertGroup(String.valueOf(id));
+        if (hasAssociatedAlertGroup) {
+            putMsg(result, Status.DELETE_ALERT_PLUGIN_INSTANCE_ERROR_HAS_ALERT_GROUP_ASSOCIATED);
+            return result;
+        }
+
         int i = alertPluginInstanceMapper.deleteById(id);
         if (i > 0) {
             putMsg(result, Status.SUCCESS);
@@ -205,4 +218,14 @@ public class AlertPluginInstanceServiceImpl extends BaseService implements Alert
         return alertPluginInstanceVOS;
 
     }
+
+    private boolean checkHasAssociatedAlertGroup(String id) {
+        List<String> idsList = alertGroupMapper.queryInstanceIdsList();
+        if (CollectionUtils.isEmpty(idsList)) {
+            return false;
+        }
+        Optional<String> first = idsList.stream().filter(k -> null != k && Arrays.asList(k.split(",")).contains(id)).findFirst();
+        return first.isPresent();
+    }
+
 }

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

@@ -0,0 +1,99 @@
+/*
+ * 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.api.enums.Status;
+import org.apache.dolphinscheduler.api.service.impl.AlertPluginInstanceServiceImpl;
+import org.apache.dolphinscheduler.common.Constants;
+import org.apache.dolphinscheduler.common.enums.UserType;
+import org.apache.dolphinscheduler.dao.entity.AlertPluginInstance;
+import org.apache.dolphinscheduler.dao.entity.User;
+import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper;
+import org.apache.dolphinscheduler.dao.mapper.AlertPluginInstanceMapper;
+import org.apache.dolphinscheduler.dao.mapper.PluginDefineMapper;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class AlertPluginInstanceServiceTest {
+
+    @InjectMocks
+    AlertPluginInstanceServiceImpl alertPluginInstanceService;
+
+    @Mock
+    private AlertPluginInstanceMapper alertPluginInstanceMapper;
+
+    @Mock
+    private PluginDefineMapper pluginDefineMapper;
+
+    @Mock
+    private AlertGroupMapper alertGroupMapper;
+
+    private List<AlertPluginInstance> alertPluginInstances;
+
+    private User user;
+
+    @Before
+    public void before() {
+        user = new User();
+        user.setUserType(UserType.ADMIN_USER);
+        user.setId(1);
+        AlertPluginInstance alertPluginInstance = new AlertPluginInstance();
+        alertPluginInstance.setPluginInstanceParams("test1");
+        alertPluginInstance.setPluginDefineId(1);
+        alertPluginInstance.setId(1);
+        alertPluginInstance.setPluginInstanceParams("test");
+        alertPluginInstances = new ArrayList<>();
+        alertPluginInstances.add(alertPluginInstance);
+    }
+
+    @Test
+    public void testCreate() {
+        Mockito.when(alertPluginInstanceMapper.queryByInstanceName("test")).thenReturn(alertPluginInstances);
+        Map<String, Object> result = alertPluginInstanceService.create(user, 1, "test", "test params");
+        Assert.assertEquals(Status.PLUGIN_INSTANCE_ALREADY_EXIT, result.get(Constants.STATUS));
+        Mockito.when(alertPluginInstanceMapper.insert(Mockito.any())).thenReturn(1);
+        result = alertPluginInstanceService.create(user, 1, "test1", "test params");
+        Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
+    }
+
+    @Test
+    public void testDelete() {
+        List<String> ids = Arrays.asList("11,2,3", null, "98,1");
+        Mockito.when(alertGroupMapper.queryInstanceIdsList()).thenReturn(ids);
+        Map<String, Object> result = alertPluginInstanceService.delete(user, 1);
+        Assert.assertEquals(Status.DELETE_ALERT_PLUGIN_INSTANCE_ERROR_HAS_ALERT_GROUP_ASSOCIATED, result.get(Constants.STATUS));
+        Mockito.when(alertPluginInstanceMapper.deleteById(9)).thenReturn(1);
+        result = alertPluginInstanceService.delete(user, 9);
+        Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
+
+    }
+
+}

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/entity/AlertPluginInstance.java

@@ -19,6 +19,7 @@ package org.apache.dolphinscheduler.dao.entity;
 
 import java.util.Date;
 
+import com.baomidou.mybatisplus.annotation.FieldStrategy;
 import com.baomidou.mybatisplus.annotation.IdType;
 import com.baomidou.mybatisplus.annotation.TableField;
 import com.baomidou.mybatisplus.annotation.TableId;
@@ -39,7 +40,7 @@ public class AlertPluginInstance {
     /**
      * plugin_define_id
      */
-    @TableField("plugin_define_id")
+    @TableField(value = "plugin_define_id", updateStrategy = FieldStrategy.NEVER)
     private int pluginDefineId;
 
     /**

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/AlertGroupMapper.java

@@ -61,6 +61,12 @@ public interface AlertGroupMapper extends BaseMapper<AlertGroup> {
      */
     List<AlertGroup> queryAllGroupList();
 
+    /**
+     * query instance ids All
+     * @return list
+     */
+    List<String> queryInstanceIdsList();
+
     /**
      * queryAlertGroupInstanceIdsById
      * @param alertGroupId

dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/AlertGroupMapper.xml

@@ -52,6 +52,13 @@
         order by update_time desc
     </select>
 
+    <select id="queryInstanceIdsList" resultType="String">
+        select
+        alert_instance_ids
+        from t_ds_alertgroup
+        order by update_time desc
+    </select>
+
     <select id="queryAlertGroupInstanceIdsById" resultType="String">
         select alert_instance_ids  from t_ds_alertgroup
         where id = #{alertGroupId}

pom.xml

@@ -787,6 +787,7 @@
                         <include>**/api/service/BaseDAGServiceTest.java</include>
                         <include>**/api/service/BaseServiceTest.java</include>
                         <include>**/api/service/DataAnalysisServiceTest.java</include>
+                        <include>**/api/service/AlertPluginInstanceServiceTest.java</include>
                         <include>**/api/service/DataSourceServiceTest.java</include>
                         <include>**/api/service/ExecutorService2Test.java</include>
                         <include>**/api/service/ExecutorServiceTest.java</include>