Merge pull request #12321 from thingsboard/fix/alarm-notification-clear

Fix duplicate alarm notification when clear rule is met

application/src/main/java/org/thingsboard/server/service/notification/rule/DefaultNotificationRuleProcessor.java

@@ -54,7 +54,6 @@ import java.util.EnumMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
-import java.util.stream.Collectors;
 
 @Service
 @RequiredArgsConstructor
@@ -106,22 +105,11 @@ public class DefaultNotificationRuleProcessor implements NotificationRuleProcess
         log.debug("Processing notification rule '{}' for trigger type {}", rule.getName(), rule.getTriggerType());
 
         if (matchesClearRule(trigger, triggerConfig)) {
-            List<NotificationRequest> notificationRequests = findAlreadySentNotificationRequests(rule, trigger);
+            List<NotificationRequest> scheduledRequests = notificationRequestService.findNotificationRequestsByRuleIdAndOriginatorEntityIdAndStatus(
-            if (notificationRequests.isEmpty()) {
+                    rule.getTenantId(), rule.getId(), trigger.getOriginatorEntityId(), NotificationRequestStatus.SCHEDULED
-                return;
+            );
-            }
+            scheduledRequests.forEach(notificationRequest -> {
-
+                notificationCenter.deleteNotificationRequest(rule.getTenantId(), notificationRequest.getId());
-            List<UUID> targets = notificationRequests.stream()
-                    .filter(NotificationRequest::isSent)
-                    .flatMap(notificationRequest -> notificationRequest.getTargets().stream())
-                    .distinct().collect(Collectors.toList());
-            NotificationInfo notificationInfo = constructNotificationInfo(trigger, triggerConfig);
-            submitNotificationRequest(targets, rule, trigger.getOriginatorEntityId(), notificationInfo, 0);
-
-            notificationRequests.forEach(notificationRequest -> {
-                if (notificationRequest.isScheduled()) {
-                    notificationCenter.deleteNotificationRequest(rule.getTenantId(), notificationRequest.getId());
-                }
             });
             return;
         }
@@ -139,10 +127,6 @@ public class DefaultNotificationRuleProcessor implements NotificationRuleProcess
         }
     }
 
-    private List<NotificationRequest> findAlreadySentNotificationRequests(NotificationRule rule, NotificationRuleTrigger trigger) {
-        return notificationRequestService.findNotificationRequestsByRuleIdAndOriginatorEntityId(rule.getTenantId(), rule.getId(), trigger.getOriginatorEntityId());
-    }
-
     private void submitNotificationRequest(List<UUID> targets, NotificationRule rule,
                                            EntityId originatorEntityId, NotificationInfo notificationInfo, int delayInSec) {
         NotificationRequestConfig config = new NotificationRequestConfig();

application/src/main/java/org/thingsboard/server/service/notification/rule/trigger/AlarmTriggerProcessor.java

@@ -46,10 +46,10 @@ public class AlarmTriggerProcessor implements NotificationRuleTriggerProcessor<A
             if (triggerConfig.getNotifyOn().contains(AlarmAction.CREATED)) {
                 return severityMatches(alarm, triggerConfig);
             }
-        }  else if (alarmUpdate.isSeverityChanged()) {
+        } else if (alarmUpdate.isSeverityChanged()) {
             if (triggerConfig.getNotifyOn().contains(AlarmAction.SEVERITY_CHANGED)) {
                 return severityMatches(alarmUpdate.getOld(), triggerConfig) || severityMatches(alarm, triggerConfig);
-            }  else {
+            } else {
                 // if we haven't yet sent notification about the alarm
                 return !severityMatches(alarmUpdate.getOld(), triggerConfig) && severityMatches(alarm, triggerConfig);
             }

application/src/test/java/org/thingsboard/server/service/notification/NotificationRuleApiTest.java

@@ -274,7 +274,6 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest {
         Device device = createDevice("Device with alarm", "233");
         NotificationTarget target = createNotificationTarget(tenantAdminUserId);
         defaultNotifications.create(tenantId, DefaultNotifications.newAlarm, target.getId());
-        defaultNotifications.create(tenantId, DefaultNotifications.entityAction, target.getId());
         notificationRulesCache.evict(tenantId);
 
         Alarm alarm = new Alarm();
@@ -287,7 +286,7 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest {
                 .pollDelay(2, TimeUnit.SECONDS)
                 .untilAsserted(() -> {
                     List<Notification> notifications = getMyNotifications(false, 10);
-                    assertThat(notifications).hasSize(1).first().matches(notification -> {
+                    assertThat(notifications).singleElement().matches(notification -> {
                         return notification.getType() == NotificationType.ALARM &&
                                 notification.getSubject().equals("New alarm 'testAlarm'");
                     });
@@ -353,12 +352,7 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest {
                 .findFirst().orElse(null);
         assertThat(scheduledNotificationRequest).extracting(NotificationRequest::getInfo).isEqualTo(notification.getInfo());
 
-        getWsClient().registerWaitForUpdate();
         alarmSubscriptionService.clearAlarm(tenantId, alarm.getId(), System.currentTimeMillis(), null);
-        getWsClient().waitForUpdate(true);
-        notification = getWsClient().getLastDataUpdate().getUpdate();
-        assertThat(notification.getSubject()).isEqualTo("critical alarm '" + alarmType + "' is CLEARED_UNACK");
-
         await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
             assertThat(findNotificationRequests(EntityType.ALARM).getData()).filteredOn(NotificationRequest::isScheduled).isEmpty();
         });

common/dao-api/src/main/java/org/thingsboard/server/dao/notification/NotificationRequestService.java

@@ -43,7 +43,7 @@ public interface NotificationRequestService {
 
     List<NotificationRequestId> findNotificationRequestsIdsByStatusAndRuleId(TenantId tenantId, NotificationRequestStatus requestStatus, NotificationRuleId ruleId);
 
-    List<NotificationRequest> findNotificationRequestsByRuleIdAndOriginatorEntityId(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId);
+    List<NotificationRequest> findNotificationRequestsByRuleIdAndOriginatorEntityIdAndStatus(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId, NotificationRequestStatus status);
 
     void deleteNotificationRequest(TenantId tenantId, NotificationRequest request);
 

dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java

@@ -82,8 +82,8 @@ public class DefaultNotificationRequestService implements NotificationRequestSer
     }
 
     @Override
-    public List<NotificationRequest> findNotificationRequestsByRuleIdAndOriginatorEntityId(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId) {
+    public List<NotificationRequest> findNotificationRequestsByRuleIdAndOriginatorEntityIdAndStatus(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId, NotificationRequestStatus status) {
-        return notificationRequestDao.findByRuleIdAndOriginatorEntityId(tenantId, ruleId, originatorEntityId);
+        return notificationRequestDao.findByRuleIdAndOriginatorEntityIdAndStatus(tenantId, ruleId, originatorEntityId, status);
     }
 
     @Override

dao/src/main/java/org/thingsboard/server/dao/notification/NotificationRequestDao.java

@@ -40,7 +40,7 @@ public interface NotificationRequestDao extends Dao<NotificationRequest> {
 
     List<NotificationRequestId> findIdsByRuleId(TenantId tenantId, NotificationRequestStatus requestStatus, NotificationRuleId ruleId);
 
-    List<NotificationRequest> findByRuleIdAndOriginatorEntityId(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId);
+    List<NotificationRequest> findByRuleIdAndOriginatorEntityIdAndStatus(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId, NotificationRequestStatus status);
 
     PageData<NotificationRequest> findAllByStatus(NotificationRequestStatus status, PageLink pageLink);
 

dao/src/main/java/org/thingsboard/server/dao/sql/notification/JpaNotificationRequestDao.java

@@ -73,8 +73,8 @@ public class JpaNotificationRequestDao extends JpaAbstractDao<NotificationReques
     }
 
     @Override
-    public List<NotificationRequest> findByRuleIdAndOriginatorEntityId(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId) {
+    public List<NotificationRequest> findByRuleIdAndOriginatorEntityIdAndStatus(TenantId tenantId, NotificationRuleId ruleId, EntityId originatorEntityId, NotificationRequestStatus status) {
-        return DaoUtil.convertDataList(notificationRequestRepository.findAllByRuleIdAndOriginatorEntityIdAndOriginatorEntityType(ruleId.getId(), originatorEntityId.getId(), originatorEntityId.getEntityType()));
+        return DaoUtil.convertDataList(notificationRequestRepository.findAllByRuleIdAndOriginatorEntityIdAndOriginatorEntityTypeAndStatus(ruleId.getId(), originatorEntityId.getId(), originatorEntityId.getEntityType(), status));
     }
 
     @Override

dao/src/main/java/org/thingsboard/server/dao/sql/notification/NotificationRequestRepository.java

@@ -54,7 +54,7 @@ public interface NotificationRequestRepository extends JpaRepository<Notificatio
     List<UUID> findAllIdsByStatusAndRuleId(@Param("status") NotificationRequestStatus status,
                                            @Param("ruleId") UUID ruleId);
 
-    List<NotificationRequestEntity> findAllByRuleIdAndOriginatorEntityIdAndOriginatorEntityType(UUID ruleId, UUID originatorEntityId, EntityType originatorEntityType);
+    List<NotificationRequestEntity> findAllByRuleIdAndOriginatorEntityIdAndOriginatorEntityTypeAndStatus(UUID ruleId, UUID originatorEntityId, EntityType originatorEntityType, NotificationRequestStatus status);
 
     Page<NotificationRequestEntity> findAllByStatus(NotificationRequestStatus status, Pageable pageable);