From e558725ffa6955c394b604dbf5019ed68d86defe Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Thu, 18 May 2023 18:51:01 +0300 Subject: [PATCH] DefaultNotificationCenter - less async operations --- .../DefaultNotificationCenter.java | 102 +++++++----------- .../channels/EmailNotificationChannel.java | 20 ++-- .../channels/NotificationChannel.java | 3 +- .../channels/SlackNotificationChannel.java | 10 +- .../channels/SmsNotificationChannel.java | 13 +-- .../NotificationRequestStats.java | 3 + 6 files changed, 56 insertions(+), 95 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/notification/DefaultNotificationCenter.java b/application/src/main/java/org/thingsboard/server/service/notification/DefaultNotificationCenter.java index 83f04f9d41..02262e2bfb 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/DefaultNotificationCenter.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/DefaultNotificationCenter.java @@ -15,13 +15,10 @@ */ package org.thingsboard.server.service.notification; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; -import org.thingsboard.common.util.DonAsynchron; import org.thingsboard.rule.engine.api.NotificationCenter; import org.thingsboard.server.common.data.EntityType; import org.thingsboard.server.common.data.User; @@ -31,6 +28,7 @@ import org.thingsboard.server.common.data.id.NotificationRuleId; import org.thingsboard.server.common.data.id.NotificationTargetId; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; +import org.thingsboard.server.common.data.limit.LimitedApi; import org.thingsboard.server.common.data.notification.AlreadySentException; import org.thingsboard.server.common.data.notification.Notification; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; @@ -59,14 +57,11 @@ import org.thingsboard.server.dao.notification.NotificationService; import org.thingsboard.server.dao.notification.NotificationSettingsService; import org.thingsboard.server.dao.notification.NotificationTargetService; import org.thingsboard.server.dao.notification.NotificationTemplateService; -import org.thingsboard.server.dao.user.UserService; +import org.thingsboard.server.dao.util.limits.RateLimitService; import org.thingsboard.server.gen.transport.TransportProtos; import org.thingsboard.server.queue.common.TbProtoQueueMsg; import org.thingsboard.server.queue.discovery.NotificationsTopicService; import org.thingsboard.server.queue.provider.TbQueueProducerProvider; -import org.thingsboard.server.common.data.limit.LimitedApi; -import org.thingsboard.server.dao.util.limits.RateLimitService; -import org.thingsboard.server.service.executors.DbCallbackExecutorService; import org.thingsboard.server.service.executors.NotificationExecutorService; import org.thingsboard.server.service.notification.channels.NotificationChannel; import org.thingsboard.server.service.subscription.TbSubscriptionUtils; @@ -74,7 +69,6 @@ import org.thingsboard.server.service.telemetry.AbstractSubscriptionService; import org.thingsboard.server.service.ws.notification.sub.NotificationRequestUpdate; import org.thingsboard.server.service.ws.notification.sub.NotificationUpdate; -import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -87,7 +81,7 @@ import java.util.stream.Collectors; @Service @Slf4j @RequiredArgsConstructor -@SuppressWarnings({"UnstableApiUsage", "rawtypes"}) +@SuppressWarnings({"rawtypes"}) public class DefaultNotificationCenter extends AbstractSubscriptionService implements NotificationCenter, NotificationChannel { private final NotificationTargetService notificationTargetService; @@ -97,7 +91,6 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple private final NotificationSettingsService notificationSettingsService; private final UserService userService; private final NotificationExecutorService notificationExecutor; - private final DbCallbackExecutorService dbCallbackExecutorService; private final NotificationsTopicService notificationsTopicService; private final TbQueueProducerProvider producerProvider; private final RateLimitService rateLimitService; @@ -172,37 +165,32 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple .build(); notificationExecutor.submit(() -> { - List> results = new ArrayList<>(); - for (NotificationTarget target : targets) { - List> result = processForTarget(target, ctx); - results.addAll(result); + processForTarget(target, ctx); + } + + NotificationRequestId requestId = ctx.getRequest().getId(); + log.debug("[{}] Notification request processing is finished", requestId); + NotificationRequestStats stats = ctx.getStats(); + try { + notificationRequestService.updateNotificationRequest(tenantId, requestId, NotificationRequestStatus.SENT, stats); + } catch (Exception e) { + log.error("[{}] Failed to update stats for notification request", requestId, e); } - Futures.whenAllComplete(results).run(() -> { - NotificationRequestId requestId = ctx.getRequest().getId(); - log.debug("[{}] Notification request processing is finished", requestId); - NotificationRequestStats stats = ctx.getStats(); + if (callback != null) { try { - notificationRequestService.updateNotificationRequest(tenantId, requestId, NotificationRequestStatus.SENT, stats); + callback.accept(stats); } catch (Exception e) { - log.error("[{}] Failed to update stats for notification request", requestId, e); - } - - if (callback != null) { - try { - callback.accept(stats); - } catch (Exception e) { - log.error("Failed to process callback for notification request {}", requestId, e); - } + log.error("Failed to process callback for notification request {}", requestId, e); } - }, dbCallbackExecutorService); + } }); return request; } - private List> processForTarget(NotificationTarget target, NotificationProcessingContext ctx) { + private void processForTarget(NotificationTarget target, NotificationProcessingContext ctx) { Iterable recipients; switch (target.getConfiguration().getType()) { case PLATFORM_USERS: { @@ -231,43 +219,35 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple Set deliveryMethods = new HashSet<>(ctx.getDeliveryMethods()); deliveryMethods.removeIf(deliveryMethod -> !target.getConfiguration().getType().getSupportedDeliveryMethods().contains(deliveryMethod)); log.debug("[{}] Processing notification request for {} target ({}) for delivery methods {}", ctx.getRequest().getId(), target.getConfiguration().getType(), target.getId(), deliveryMethods); + if (deliveryMethods.isEmpty()) { + return; + } - List> results = new ArrayList<>(); - if (!deliveryMethods.isEmpty()) { - for (NotificationRecipient recipient : recipients) { - for (NotificationDeliveryMethod deliveryMethod : deliveryMethods) { - ListenableFuture resultFuture = processForRecipient(deliveryMethod, recipient, ctx); - DonAsynchron.withCallback(resultFuture, result -> { - ctx.getStats().reportSent(deliveryMethod, recipient); - }, error -> { - ctx.getStats().reportError(deliveryMethod, error, recipient); - }); - results.add(resultFuture); + for (NotificationRecipient recipient : recipients) { + for (NotificationDeliveryMethod deliveryMethod : deliveryMethods) { + try { + processForRecipient(deliveryMethod, recipient, ctx); + ctx.getStats().reportSent(deliveryMethod, recipient); + } catch (Exception error) { + ctx.getStats().reportError(deliveryMethod, error, recipient); } } } - return results; } - private ListenableFuture processForRecipient(NotificationDeliveryMethod deliveryMethod, NotificationRecipient recipient, NotificationProcessingContext ctx) { + private void processForRecipient(NotificationDeliveryMethod deliveryMethod, NotificationRecipient recipient, NotificationProcessingContext ctx) throws Exception { if (ctx.getStats().contains(deliveryMethod, recipient.getId())) { - return Futures.immediateFailedFuture(new AlreadySentException()); + throw new AlreadySentException(); } - - DeliveryMethodNotificationTemplate processedTemplate; - try { - processedTemplate = ctx.getProcessedTemplate(deliveryMethod, recipient); - } catch (Exception e) { - return Futures.immediateFailedFuture(e); - } - NotificationChannel notificationChannel = channels.get(deliveryMethod); + DeliveryMethodNotificationTemplate processedTemplate = ctx.getProcessedTemplate(deliveryMethod, recipient); + log.trace("[{}] Sending {} notification for recipient {}", ctx.getRequest().getId(), deliveryMethod, recipient); - return notificationChannel.sendNotification(recipient, processedTemplate, ctx); + notificationChannel.sendNotification(recipient, processedTemplate, ctx); } @Override - public ListenableFuture sendNotification(User recipient, WebDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) { + public void sendNotification(User recipient, WebDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) throws Exception { NotificationRequest request = ctx.getRequest(); Notification notification = Notification.builder() .requestId(request.getId()) @@ -283,14 +263,14 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple notification = notificationService.saveNotification(recipient.getTenantId(), notification); } catch (Exception e) { log.error("Failed to create notification for recipient {}", recipient.getId(), e); - return Futures.immediateFailedFuture(e); + throw e; } NotificationUpdate update = NotificationUpdate.builder() .created(true) .notification(notification) .build(); - return onNotificationUpdate(recipient.getTenantId(), recipient.getId(), update); + onNotificationUpdate(recipient.getTenantId(), recipient.getId(), update); } @Override @@ -384,13 +364,11 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple clusterService.pushMsgToCore(tenantId, notificationRequestId, toCoreMsg, null); } - private ListenableFuture onNotificationUpdate(TenantId tenantId, UserId recipientId, NotificationUpdate update) { + private void onNotificationUpdate(TenantId tenantId, UserId recipientId, NotificationUpdate update) { log.trace("Submitting notification update for recipient {}: {}", recipientId, update); - return Futures.submit(() -> { - forwardToSubscriptionManagerService(tenantId, recipientId, subscriptionManagerService -> { - subscriptionManagerService.onNotificationUpdate(tenantId, recipientId, update, TbCallback.EMPTY); - }, () -> TbSubscriptionUtils.notificationUpdateToProto(tenantId, recipientId, update)); - }, wsCallBackExecutor); + forwardToSubscriptionManagerService(tenantId, recipientId, subscriptionManagerService -> { + subscriptionManagerService.onNotificationUpdate(tenantId, recipientId, update, TbCallback.EMPTY); + }, () -> TbSubscriptionUtils.notificationUpdateToProto(tenantId, recipientId, update)); } private void onNotificationRequestUpdate(TenantId tenantId, NotificationRequestUpdate update) { diff --git a/application/src/main/java/org/thingsboard/server/service/notification/channels/EmailNotificationChannel.java b/application/src/main/java/org/thingsboard/server/service/notification/channels/EmailNotificationChannel.java index 8b3c9551c2..8e9e8525e3 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/channels/EmailNotificationChannel.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/channels/EmailNotificationChannel.java @@ -15,7 +15,6 @@ */ package org.thingsboard.server.service.notification.channels; -import com.google.common.util.concurrent.ListenableFuture; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Component; import org.thingsboard.rule.engine.api.MailService; @@ -24,7 +23,6 @@ import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; import org.thingsboard.server.common.data.notification.template.EmailDeliveryMethodNotificationTemplate; -import org.thingsboard.server.service.mail.MailExecutorService; import org.thingsboard.server.service.notification.NotificationProcessingContext; @Component @@ -32,19 +30,15 @@ import org.thingsboard.server.service.notification.NotificationProcessingContext public class EmailNotificationChannel implements NotificationChannel { private final MailService mailService; - private final MailExecutorService executor; @Override - public ListenableFuture sendNotification(User recipient, EmailDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) { - return executor.submit(() -> { - mailService.send(recipient.getTenantId(), null, TbEmail.builder() - .to(recipient.getEmail()) - .subject(processedTemplate.getSubject()) - .body(processedTemplate.getBody()) - .html(true) - .build()); - return null; - }); + public void sendNotification(User recipient, EmailDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) throws Exception { + mailService.send(recipient.getTenantId(), null, TbEmail.builder() + .to(recipient.getEmail()) + .subject(processedTemplate.getSubject()) + .body(processedTemplate.getBody()) + .html(true) + .build()); } @Override diff --git a/application/src/main/java/org/thingsboard/server/service/notification/channels/NotificationChannel.java b/application/src/main/java/org/thingsboard/server/service/notification/channels/NotificationChannel.java index 02fe6264d2..0275644765 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/channels/NotificationChannel.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/channels/NotificationChannel.java @@ -15,7 +15,6 @@ */ package org.thingsboard.server.service.notification.channels; -import com.google.common.util.concurrent.ListenableFuture; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; import org.thingsboard.server.common.data.notification.targets.NotificationRecipient; @@ -24,7 +23,7 @@ import org.thingsboard.server.service.notification.NotificationProcessingContext public interface NotificationChannel { - ListenableFuture sendNotification(R recipient, T processedTemplate, NotificationProcessingContext ctx); + void sendNotification(R recipient, T processedTemplate, NotificationProcessingContext ctx) throws Exception; void check(TenantId tenantId) throws Exception; diff --git a/application/src/main/java/org/thingsboard/server/service/notification/channels/SlackNotificationChannel.java b/application/src/main/java/org/thingsboard/server/service/notification/channels/SlackNotificationChannel.java index 46afbd7270..25c6b9dcab 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/channels/SlackNotificationChannel.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/channels/SlackNotificationChannel.java @@ -15,7 +15,6 @@ */ package org.thingsboard.server.service.notification.channels; -import com.google.common.util.concurrent.ListenableFuture; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Component; import org.thingsboard.rule.engine.api.slack.SlackService; @@ -26,7 +25,6 @@ import org.thingsboard.server.common.data.notification.settings.SlackNotificatio import org.thingsboard.server.common.data.notification.targets.slack.SlackConversation; import org.thingsboard.server.common.data.notification.template.SlackDeliveryMethodNotificationTemplate; import org.thingsboard.server.dao.notification.NotificationSettingsService; -import org.thingsboard.server.service.executors.ExternalCallExecutorService; import org.thingsboard.server.service.notification.NotificationProcessingContext; @Component @@ -35,15 +33,11 @@ public class SlackNotificationChannel implements NotificationChannel sendNotification(SlackConversation conversation, SlackDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) { + public void sendNotification(SlackConversation conversation, SlackDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) throws Exception { SlackNotificationDeliveryMethodConfig config = ctx.getDeliveryMethodConfig(NotificationDeliveryMethod.SLACK); - return executor.submit(() -> { - slackService.sendMessage(ctx.getTenantId(), config.getBotToken(), conversation.getId(), processedTemplate.getBody()); - return null; - }); + slackService.sendMessage(ctx.getTenantId(), config.getBotToken(), conversation.getId(), processedTemplate.getBody()); } @Override diff --git a/application/src/main/java/org/thingsboard/server/service/notification/channels/SmsNotificationChannel.java b/application/src/main/java/org/thingsboard/server/service/notification/channels/SmsNotificationChannel.java index 3c44dabd4b..44b61d3bb3 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/channels/SmsNotificationChannel.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/channels/SmsNotificationChannel.java @@ -15,8 +15,6 @@ */ package org.thingsboard.server.service.notification.channels; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; import lombok.RequiredArgsConstructor; import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Component; @@ -26,26 +24,21 @@ import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; import org.thingsboard.server.common.data.notification.template.SmsDeliveryMethodNotificationTemplate; import org.thingsboard.server.service.notification.NotificationProcessingContext; -import org.thingsboard.server.service.sms.SmsExecutorService; @Component @RequiredArgsConstructor public class SmsNotificationChannel implements NotificationChannel { private final SmsService smsService; - private final SmsExecutorService executor; @Override - public ListenableFuture sendNotification(User recipient, SmsDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) { + public void sendNotification(User recipient, SmsDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) throws Exception { String phone = recipient.getPhone(); if (StringUtils.isBlank(phone)) { - return Futures.immediateFailedFuture(new RuntimeException("User does not have phone number")); + throw new RuntimeException("User does not have phone number"); } - return executor.submit(() -> { - smsService.sendSms(recipient.getTenantId(), recipient.getCustomerId(), new String[]{phone}, processedTemplate.getBody()); - return null; - }); + smsService.sendSms(recipient.getTenantId(), recipient.getCustomerId(), new String[]{phone}, processedTemplate.getBody()); } @Override diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/NotificationRequestStats.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/NotificationRequestStats.java index 31e05a2cb3..619e1ad38f 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/NotificationRequestStats.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/NotificationRequestStats.java @@ -62,6 +62,9 @@ public class NotificationRequestStats { return; } String errorMessage = error.getMessage(); + if (errorMessage == null) { + errorMessage = error.getClass().getSimpleName(); + } errors.computeIfAbsent(deliveryMethod, k -> new ConcurrentHashMap<>()).put(recipient.getTitle(), errorMessage); }