From 5a57657479c833fbc87c00a2be2b2c9b3d05aa53 Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Wed, 12 Apr 2023 15:14:38 +0300 Subject: [PATCH 1/5] Notification preview for first recipient; Slack notification preview; increase timeouts for testDelayedNotificationRequest --- .../controller/NotificationController.java | 53 ++++++----- .../NotificationTargetController.java | 2 +- .../DefaultNotificationCenter.java | 2 +- .../notification/NotificationApiTest.java | 94 ++++++++++--------- .../targets/NotificationTargetType.java | 7 ++ 5 files changed, 84 insertions(+), 74 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/NotificationController.java b/application/src/main/java/org/thingsboard/server/controller/NotificationController.java index 6c27887e1a..d36eb9913d 100644 --- a/application/src/main/java/org/thingsboard/server/controller/NotificationController.java +++ b/application/src/main/java/org/thingsboard/server/controller/NotificationController.java @@ -64,7 +64,9 @@ import org.thingsboard.server.service.security.permission.Operation; import org.thingsboard.server.service.security.permission.Resource; import javax.validation.Valid; +import java.util.Comparator; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -201,9 +203,6 @@ public class NotificationController extends BaseController { public NotificationRequestPreview getNotificationRequestPreview(@RequestBody @Valid NotificationRequest request, @RequestParam(defaultValue = "20") int recipientsPreviewSize, @AuthenticationPrincipal SecurityUser user) throws ThingsboardException { - NotificationRequestPreview preview = new NotificationRequestPreview(); - - request.setOriginatorEntityId(user.getId()); NotificationTemplate template; if (request.getTemplateId() != null) { template = checkEntityId(request.getTemplateId(), notificationTemplateService::findNotificationTemplateById, Operation.READ); @@ -213,33 +212,23 @@ public class NotificationController extends BaseController { if (template == null) { throw new IllegalArgumentException("Template is missing"); } - NotificationProcessingContext tmpProcessingCtx = NotificationProcessingContext.builder() - .tenantId(user.getTenantId()) - .request(request) - .template(template) - .settings(null) - .build(); + request.setOriginatorEntityId(user.getId()); + List targets = request.getTargets().stream() + .map(NotificationTargetId::new) + .map(targetId -> notificationTargetService.findNotificationTargetById(user.getTenantId(), targetId)) + .sorted(Comparator.comparing(target -> target.getConfiguration().getType())) + .collect(Collectors.toList()); - Map processedTemplates = tmpProcessingCtx.getDeliveryMethods().stream() - .collect(Collectors.toMap(m -> m, deliveryMethod -> { - NotificationRecipient recipient = null; - if (NotificationTargetType.PLATFORM_USERS.getSupportedDeliveryMethods().contains(deliveryMethod)) { - recipient = userService.findUserById(user.getTenantId(), user.getId()); - } - return tmpProcessingCtx.getProcessedTemplate(deliveryMethod, recipient); - })); - preview.setProcessedTemplates(processedTemplates); + NotificationRequestPreview preview = new NotificationRequestPreview(); - // generic permission Set recipientsPreview = new LinkedHashSet<>(); - Map recipientsCountByTarget = new HashMap<>(); - - List targets = notificationTargetService.findNotificationTargetsByTenantIdAndIds(user.getTenantId(), - request.getTargets().stream().map(NotificationTargetId::new).collect(Collectors.toList())); + Map recipientsCountByTarget = new LinkedHashMap<>(); + Map firstRecipient = new HashMap<>(); for (NotificationTarget target : targets) { int recipientsCount; List recipientsPart; - if (target.getConfiguration().getType() == NotificationTargetType.PLATFORM_USERS) { + NotificationTargetType targetType = target.getConfiguration().getType(); + if (targetType == NotificationTargetType.PLATFORM_USERS) { PageData recipients = notificationTargetService.findRecipientsForNotificationTargetConfig(user.getTenantId(), (PlatformUsersNotificationTargetConfig) target.getConfiguration(), new PageLink(recipientsPreviewSize)); recipientsCount = (int) recipients.getTotalElements(); @@ -248,7 +237,7 @@ public class NotificationController extends BaseController { recipientsCount = 1; recipientsPart = List.of(((SlackNotificationTargetConfig) target.getConfiguration()).getConversation()); } - + firstRecipient.putIfAbsent(targetType, !recipientsPart.isEmpty() ? recipientsPart.get(0) : null); for (NotificationRecipient recipient : recipientsPart) { if (recipientsPreview.size() < recipientsPreviewSize) { recipientsPreview.add(recipient.getTitle()); @@ -258,11 +247,23 @@ public class NotificationController extends BaseController { } recipientsCountByTarget.put(target.getName(), recipientsCount); } - preview.setRecipientsPreview(recipientsPreview); preview.setRecipientsCountByTarget(recipientsCountByTarget); preview.setTotalRecipientsCount(recipientsCountByTarget.values().stream().mapToInt(Integer::intValue).sum()); + NotificationProcessingContext ctx = NotificationProcessingContext.builder() + .tenantId(user.getTenantId()) + .request(request) + .template(template) + .settings(null) + .build(); + Map processedTemplates = ctx.getDeliveryMethods().stream() + .collect(Collectors.toMap(m -> m, deliveryMethod -> { + NotificationTargetType targetType = NotificationTargetType.forDeliveryMethod(deliveryMethod); + return ctx.getProcessedTemplate(deliveryMethod, firstRecipient.get(targetType)); + })); + preview.setProcessedTemplates(processedTemplates); + return preview; } diff --git a/application/src/main/java/org/thingsboard/server/controller/NotificationTargetController.java b/application/src/main/java/org/thingsboard/server/controller/NotificationTargetController.java index 4fd6b93022..6374b65710 100644 --- a/application/src/main/java/org/thingsboard/server/controller/NotificationTargetController.java +++ b/application/src/main/java/org/thingsboard/server/controller/NotificationTargetController.java @@ -71,7 +71,7 @@ public class NotificationTargetController extends BaseController { private final NotificationTargetService notificationTargetService; @ApiOperation(value = "Save notification target (saveNotificationTarget)", - notes = "Create or update notification target.\n\n" + + notes = "Create or update notification target." + SYSTEM_OR_TENANT_AUTHORITY_PARAGRAPH) @PostMapping("/target") @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN')") 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 b4fba04212..fa7e973843 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 @@ -130,7 +130,7 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple } if (ruleId == null) { if (targets.stream().noneMatch(target -> target.getConfiguration().getType().getSupportedDeliveryMethods().contains(deliveryMethod))) { - throw new IllegalArgumentException("Target for " + deliveryMethod.getName() + " delivery method is missing"); + throw new IllegalArgumentException("Recipients for " + deliveryMethod.getName() + " delivery method not chosen"); } } }); diff --git a/application/src/test/java/org/thingsboard/server/service/notification/NotificationApiTest.java b/application/src/test/java/org/thingsboard/server/service/notification/NotificationApiTest.java index 9dd34296ef..f6a296ab7e 100644 --- a/application/src/test/java/org/thingsboard/server/service/notification/NotificationApiTest.java +++ b/application/src/test/java/org/thingsboard/server/service/notification/NotificationApiTest.java @@ -40,6 +40,7 @@ import org.thingsboard.server.common.data.notification.targets.platform.Customer import org.thingsboard.server.common.data.notification.targets.platform.PlatformUsersNotificationTargetConfig; import org.thingsboard.server.common.data.notification.targets.platform.UserListFilter; import org.thingsboard.server.common.data.notification.targets.slack.SlackConversation; +import org.thingsboard.server.common.data.notification.targets.slack.SlackConversationType; import org.thingsboard.server.common.data.notification.targets.slack.SlackNotificationTargetConfig; import org.thingsboard.server.common.data.notification.template.DeliveryMethodNotificationTemplate; import org.thingsboard.server.common.data.notification.template.EmailDeliveryMethodNotificationTemplate; @@ -49,7 +50,6 @@ import org.thingsboard.server.common.data.notification.template.SlackDeliveryMet import org.thingsboard.server.common.data.notification.template.SmsDeliveryMethodNotificationTemplate; import org.thingsboard.server.common.data.notification.template.WebDeliveryMethodNotificationTemplate; import org.thingsboard.server.common.data.security.Authority; -import org.thingsboard.server.dao.DaoUtil; import org.thingsboard.server.dao.notification.NotificationDao; import org.thingsboard.server.dao.service.DaoSqlTest; import org.thingsboard.server.service.executors.DbCallbackExecutorService; @@ -226,13 +226,13 @@ public class NotificationApiTest extends AbstractNotificationApiTest { NotificationRequest notificationRequest = submitNotificationRequest(notificationTarget.getId(), notificationText, 5); assertThat(notificationRequest.getStatus()).isEqualTo(NotificationRequestStatus.SCHEDULED); await().atLeast(4, TimeUnit.SECONDS) - .atMost(6, TimeUnit.SECONDS) + .atMost(15, TimeUnit.SECONDS) .until(() -> wsClient.getLastMsg() != null); Notification delayedNotification = wsClient.getLastDataUpdate().getUpdate(); assertThat(delayedNotification).extracting(Notification::getText).isEqualTo(notificationText); assertThat(delayedNotification.getCreatedTime() - notificationRequest.getCreatedTime()) - .isCloseTo(TimeUnit.SECONDS.toMillis(5), Offset.offset(500L)); + .isCloseTo(TimeUnit.SECONDS.toMillis(5), Offset.offset(10000L)); assertThat(findNotificationRequest(notificationRequest.getId()).getStatus()).isEqualTo(NotificationRequestStatus.SENT); } @@ -324,16 +324,17 @@ public class NotificationApiTest extends AbstractNotificationApiTest { @Test public void testNotificationRequestPreview() throws Exception { - NotificationTarget target1 = new NotificationTarget(); - target1.setName("Me"); - PlatformUsersNotificationTargetConfig target1Config = new PlatformUsersNotificationTargetConfig(); + NotificationTarget tenantAdminTarget = new NotificationTarget(); + tenantAdminTarget.setName("Me"); + PlatformUsersNotificationTargetConfig tenantAdminTargetConfig = new PlatformUsersNotificationTargetConfig(); UserListFilter userListFilter = new UserListFilter(); - userListFilter.setUsersIds(DaoUtil.toUUIDs(List.of(tenantAdminUserId))); - target1Config.setUsersFilter(userListFilter); - target1.setConfiguration(target1Config); - target1 = saveNotificationTarget(target1); + userListFilter.setUsersIds(List.of(tenantAdminUserId.getId())); + tenantAdminTargetConfig.setUsersFilter(userListFilter); + tenantAdminTarget.setConfiguration(tenantAdminTargetConfig); + tenantAdminTarget = saveNotificationTarget(tenantAdminTarget); List recipients = new ArrayList<>(); recipients.add(TENANT_ADMIN_EMAIL); + String firstRecipientEmail = TENANT_ADMIN_EMAIL; createDifferentCustomer(); loginTenantAdmin(); @@ -347,21 +348,32 @@ public class NotificationApiTest extends AbstractNotificationApiTest { customerUser = createUser(customerUser, "12345678"); recipients.add(customerUser.getEmail()); } - NotificationTarget target2 = new NotificationTarget(); - target2.setName("Other customer users"); - PlatformUsersNotificationTargetConfig target2Config = new PlatformUsersNotificationTargetConfig(); + NotificationTarget customerUsersTarget = new NotificationTarget(); + customerUsersTarget.setName("Other customer users"); + PlatformUsersNotificationTargetConfig customerUsersTargetConfig = new PlatformUsersNotificationTargetConfig(); CustomerUsersFilter customerUsersFilter = new CustomerUsersFilter(); customerUsersFilter.setCustomerId(differentCustomerId.getId()); - target2Config.setUsersFilter(customerUsersFilter); - target2.setConfiguration(target2Config); - target2 = saveNotificationTarget(target2); - + customerUsersTargetConfig.setUsersFilter(customerUsersFilter); + customerUsersTarget.setConfiguration(customerUsersTargetConfig); + customerUsersTarget = saveNotificationTarget(customerUsersTarget); + + NotificationTarget slackTarget = new NotificationTarget(); + slackTarget.setName("Slack user"); + SlackNotificationTargetConfig slackTargetConfig = new SlackNotificationTargetConfig(); + slackTargetConfig.setConversationType(SlackConversationType.DIRECT); + SlackConversation slackConversation = new SlackConversation(); + slackConversation.setId("U1234567"); + slackConversation.setTitle("@jdoe (John Doe)"); + slackConversation.setWholeName("John Doe"); + slackTargetConfig.setConversation(slackConversation); + slackTarget.setConfiguration(slackTargetConfig); + slackTarget = saveNotificationTarget(slackTarget); + recipients.add(slackConversation.getTitle()); NotificationTemplate notificationTemplate = new NotificationTemplate(); notificationTemplate.setNotificationType(NotificationType.GENERAL); notificationTemplate.setName("Test template"); - String requestorEmail = TENANT_ADMIN_EMAIL; NotificationTemplateConfig templateConfig = new NotificationTemplateConfig(); HashMap templates = new HashMap<>(); templateConfig.setDeliveryMethodsTemplates(templates); @@ -369,69 +381,59 @@ public class NotificationApiTest extends AbstractNotificationApiTest { WebDeliveryMethodNotificationTemplate webNotificationTemplate = new WebDeliveryMethodNotificationTemplate(); webNotificationTemplate.setEnabled(true); - webNotificationTemplate.setBody("Message for WEB: ${recipientEmail} ${unknownParam}"); - webNotificationTemplate.setSubject("Subject for WEB: ${recipientEmail}"); + webNotificationTemplate.setSubject("WEB SUBJECT: ${recipientEmail}"); + webNotificationTemplate.setBody("WEB: ${recipientEmail} ${unknownParam}"); templates.put(NotificationDeliveryMethod.WEB, webNotificationTemplate); SmsDeliveryMethodNotificationTemplate smsNotificationTemplate = new SmsDeliveryMethodNotificationTemplate(); smsNotificationTemplate.setEnabled(true); - smsNotificationTemplate.setBody("Message for SMS: ${recipientEmail}"); + smsNotificationTemplate.setBody("SMS: ${recipientEmail}"); templates.put(NotificationDeliveryMethod.SMS, smsNotificationTemplate); EmailDeliveryMethodNotificationTemplate emailNotificationTemplate = new EmailDeliveryMethodNotificationTemplate(); emailNotificationTemplate.setEnabled(true); - emailNotificationTemplate.setSubject("Subject for EMAIL: ${recipientEmail}"); - emailNotificationTemplate.setBody("Message for EMAIL: ${recipientEmail}"); + emailNotificationTemplate.setSubject("EMAIL SUBJECT: ${recipientEmail}"); + emailNotificationTemplate.setBody("EMAIL: ${recipientEmail}"); templates.put(NotificationDeliveryMethod.EMAIL, emailNotificationTemplate); SlackDeliveryMethodNotificationTemplate slackNotificationTemplate = new SlackDeliveryMethodNotificationTemplate(); slackNotificationTemplate.setEnabled(true); - slackNotificationTemplate.setBody("Message for SLACK: ${recipientEmail}"); + slackNotificationTemplate.setBody("SLACK: ${recipientFirstName} ${recipientLastName}"); templates.put(NotificationDeliveryMethod.SLACK, slackNotificationTemplate); notificationTemplate = saveNotificationTemplate(notificationTemplate); - NotificationRequest notificationRequest = new NotificationRequest(); - notificationRequest.setTargets(List.of(target1.getUuidId(), target2.getUuidId())); + notificationRequest.setTargets(List.of(tenantAdminTarget.getUuidId(), customerUsersTarget.getUuidId(), slackTarget.getUuidId())); notificationRequest.setTemplateId(notificationTemplate.getId()); notificationRequest.setAdditionalConfig(new NotificationRequestConfig()); NotificationRequestPreview preview = doPost("/api/notification/request/preview", notificationRequest, NotificationRequestPreview.class); - assertThat(preview.getRecipientsCountByTarget().get(target1.getName())).isEqualTo(1); - assertThat(preview.getRecipientsCountByTarget().get(target2.getName())).isEqualTo(customerUsersCount); - assertThat(preview.getTotalRecipientsCount()).isEqualTo(1 + customerUsersCount); + assertThat(preview.getRecipientsCountByTarget().get(tenantAdminTarget.getName())).isEqualTo(1); + assertThat(preview.getRecipientsCountByTarget().get(customerUsersTarget.getName())).isEqualTo(customerUsersCount); + assertThat(preview.getRecipientsCountByTarget().get(slackTarget.getName())).isEqualTo(1); + + assertThat(preview.getTotalRecipientsCount()).isEqualTo(2 + customerUsersCount); assertThat(preview.getRecipientsPreview()).containsAll(recipients); Map processedTemplates = preview.getProcessedTemplates(); assertThat(processedTemplates.get(NotificationDeliveryMethod.WEB)).asInstanceOf(type(WebDeliveryMethodNotificationTemplate.class)) .satisfies(template -> { - assertThat(template.getBody()) - .startsWith("Message for WEB") - .endsWith(requestorEmail + " ${unknownParam}"); - assertThat(template.getSubject()) - .startsWith("Subject for WEB") - .endsWith(requestorEmail); + assertThat(template.getSubject()).isEqualTo("WEB SUBJECT: " + firstRecipientEmail); + assertThat(template.getBody()).isEqualTo("WEB: " + firstRecipientEmail + " ${unknownParam}"); }); assertThat(processedTemplates.get(NotificationDeliveryMethod.SMS)).asInstanceOf(type(SmsDeliveryMethodNotificationTemplate.class)) .satisfies(template -> { - assertThat(template.getBody()) - .startsWith("Message for SMS") - .endsWith(requestorEmail); + assertThat(template.getBody()).isEqualTo("SMS: " + firstRecipientEmail); }); assertThat(processedTemplates.get(NotificationDeliveryMethod.EMAIL)).asInstanceOf(type(EmailDeliveryMethodNotificationTemplate.class)) .satisfies(template -> { - assertThat(template.getBody()) - .startsWith("Message for EMAIL") - .endsWith(requestorEmail); - assertThat(template.getSubject()) - .startsWith("Subject for EMAIL") - .endsWith(requestorEmail); + assertThat(template.getSubject()).isEqualTo("EMAIL SUBJECT: " + firstRecipientEmail); + assertThat(template.getBody()).isEqualTo("EMAIL: " + firstRecipientEmail); }); assertThat(processedTemplates.get(NotificationDeliveryMethod.SLACK)).asInstanceOf(type(SlackDeliveryMethodNotificationTemplate.class)) .satisfies(template -> { - assertThat(template.getBody()) - .isEqualTo("Message for SLACK: ${recipientEmail}"); // ${recipientEmail} should not be processed + assertThat(template.getBody()).isEqualTo("SLACK: John Doe"); }); } diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetType.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetType.java index 0c2441e37d..1254654ecf 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetType.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetType.java @@ -19,6 +19,7 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; +import java.util.Arrays; import java.util.Set; @RequiredArgsConstructor @@ -30,4 +31,10 @@ public enum NotificationTargetType { @Getter private final Set supportedDeliveryMethods; + public static NotificationTargetType forDeliveryMethod(NotificationDeliveryMethod deliveryMethod) { + return Arrays.stream(values()) + .filter(targetType -> targetType.getSupportedDeliveryMethods().contains(deliveryMethod)) + .findFirst().orElse(null); + } + } From e926854911524d7063ecfecd65cd7fd68028a8fb Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Thu, 13 Apr 2023 12:53:59 +0300 Subject: [PATCH 2/5] NoXss and length validation for notification entities --- .../server/controller/BaseController.java | 22 ++++++---- .../DefaultNotificationCenter.java | 18 +++++--- .../channels/EmailNotificationChannel.java | 5 +-- .../channels/NotificationChannel.java | 2 +- .../channels/SlackNotificationChannel.java | 10 +++-- .../channels/SmsNotificationChannel.java | 6 ++- .../notification/rule/NotificationRule.java | 2 + .../targets/NotificationTarget.java | 2 + .../targets/NotificationTargetConfig.java | 2 + ...ailDeliveryMethodNotificationTemplate.java | 4 ++ .../template/NotificationTemplate.java | 2 + .../template/NotificationText.java | 30 ------------- ...ackDeliveryMethodNotificationTemplate.java | 7 +++ ...SmsDeliveryMethodNotificationTemplate.java | 9 ++++ ...WebDeliveryMethodNotificationTemplate.java | 15 +++++++ .../server/common/data/validation/Length.java | 4 +- .../server/common/data/validation/NoXss.java | 6 ++- .../dao/service/ConstraintValidator.java | 44 +++++++++++-------- 18 files changed, 115 insertions(+), 75 deletions(-) delete mode 100644 common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationText.java diff --git a/application/src/main/java/org/thingsboard/server/controller/BaseController.java b/application/src/main/java/org/thingsboard/server/controller/BaseController.java index 34f065d500..8a892b91a9 100644 --- a/application/src/main/java/org/thingsboard/server/controller/BaseController.java +++ b/application/src/main/java/org/thingsboard/server/controller/BaseController.java @@ -103,7 +103,6 @@ import org.thingsboard.server.common.data.rpc.Rpc; import org.thingsboard.server.common.data.rule.RuleChain; import org.thingsboard.server.common.data.rule.RuleChainType; import org.thingsboard.server.common.data.rule.RuleNode; -import org.thingsboard.server.common.data.settings.UserDashboardAction; import org.thingsboard.server.common.data.util.ThrowingBiFunction; import org.thingsboard.server.common.data.widget.WidgetTypeDetails; import org.thingsboard.server.common.data.widget.WidgetsBundle; @@ -130,12 +129,12 @@ import org.thingsboard.server.dao.queue.QueueService; import org.thingsboard.server.dao.relation.RelationService; import org.thingsboard.server.dao.rpc.RpcService; import org.thingsboard.server.dao.rule.RuleChainService; +import org.thingsboard.server.dao.service.ConstraintValidator; import org.thingsboard.server.dao.service.Validator; import org.thingsboard.server.dao.tenant.TbTenantProfileCache; import org.thingsboard.server.dao.tenant.TenantProfileService; import org.thingsboard.server.dao.tenant.TenantService; import org.thingsboard.server.dao.user.UserService; -import org.thingsboard.server.dao.user.UserSettingsService; import org.thingsboard.server.dao.widget.WidgetTypeService; import org.thingsboard.server.dao.widget.WidgetsBundleService; import org.thingsboard.server.exception.ThingsboardErrorResponseHandler; @@ -164,7 +163,9 @@ import org.thingsboard.server.service.telemetry.TelemetrySubscriptionService; import javax.mail.MessagingException; import javax.servlet.http.HttpServletResponse; +import javax.validation.ConstraintViolation; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.UUID; @@ -395,16 +396,19 @@ public abstract class BaseController { * Handles validation error for controller method arguments annotated with @{@link javax.validation.Valid} * */ @ExceptionHandler(MethodArgumentNotValidException.class) - public void handleValidationError(MethodArgumentNotValidException e, HttpServletResponse response) { - String errorMessage = "Validation error: " + e.getFieldErrors().stream() + public void handleValidationError(MethodArgumentNotValidException validationError, HttpServletResponse response) { + List> constraintsViolations = validationError.getFieldErrors().stream() .map(fieldError -> { - String property = fieldError.getField(); - if (property.equals("valid") || StringUtils.endsWith(property, ".valid")) { // when custom @AssertTrue is used - property = ""; + try { + return (ConstraintViolation) fieldError.unwrap(ConstraintViolation.class); + } catch (Exception e) { + log.warn("FieldError source is not of type ConstraintViolation"); + return null; // should not happen } - return (!property.isEmpty() ? (property + " ") : "") + fieldError.getDefaultMessage(); }) - .collect(Collectors.joining(", ")); + .filter(Objects::nonNull) + .collect(Collectors.toList()); + String errorMessage = "Validation error: " + ConstraintValidator.getErrorMessage(constraintsViolations); ThingsboardException thingsboardException = new ThingsboardException(errorMessage, ThingsboardErrorCode.BAD_REQUEST_PARAMS); handleControllerException(thingsboardException, response); } 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 fa7e973843..d297f70a7f 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 @@ -125,8 +125,10 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple NotificationRuleId ruleId = request.getRuleId(); notificationTemplate.getConfiguration().getDeliveryMethodsTemplates().forEach((deliveryMethod, template) -> { if (!template.isEnabled()) return; - if (!channels.get(deliveryMethod).check(tenantId)) { - throw new IllegalArgumentException("Unable to send notification via " + deliveryMethod.getName() + ": not configured or not working"); + try { + channels.get(deliveryMethod).check(tenantId); + } catch (Exception e) { + throw new IllegalArgumentException(e.getMessage()); } if (ruleId == null) { if (targets.stream().noneMatch(target -> target.getConfiguration().getType().getSupportedDeliveryMethods().contains(deliveryMethod))) { @@ -341,14 +343,20 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple @Override public Set getAvailableDeliveryMethods(TenantId tenantId) { return channels.values().stream() - .filter(channel -> channel.check(tenantId)) + .filter(channel -> { + try { + channel.check(tenantId); + return true; + } catch (Exception e) { + return false; + } + }) .map(NotificationChannel::getDeliveryMethod) .collect(Collectors.toSet()); } @Override - public boolean check(TenantId tenantId) { - return true; + public void check(TenantId tenantId) throws Exception { } @Override 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 61e4c528dc..f5bbc57954 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 @@ -48,12 +48,11 @@ public class EmailNotificationChannel implements NotificationChannel sendNotification(R recipient, T processedTemplate, NotificationProcessingContext ctx); - boolean check(TenantId tenantId); + void check(TenantId tenantId) throws Exception; NotificationDeliveryMethod getDeliveryMethod(); 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 7ff48d4cb9..46afbd7270 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 @@ -22,12 +22,12 @@ import org.thingsboard.rule.engine.api.slack.SlackService; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; import org.thingsboard.server.common.data.notification.settings.NotificationSettings; -import org.thingsboard.server.dao.notification.NotificationSettingsService; -import org.thingsboard.server.service.notification.NotificationProcessingContext; import org.thingsboard.server.common.data.notification.settings.SlackNotificationDeliveryMethodConfig; 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 @RequiredArgsConstructor @@ -47,9 +47,11 @@ public class SlackNotificationChannel implements NotificationChannel implements Ha private TenantId tenantId; @NotBlank @NoXss + @Length(max = 255, message = "cannot be longer than 255 chars") private String name; @NotNull private NotificationTemplateId templateId; diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTarget.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTarget.java index 9f267a514d..9a2f9a5306 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTarget.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTarget.java @@ -22,6 +22,7 @@ import org.thingsboard.server.common.data.HasName; import org.thingsboard.server.common.data.HasTenantId; import org.thingsboard.server.common.data.id.NotificationTargetId; import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.common.data.validation.Length; import org.thingsboard.server.common.data.validation.NoXss; import javax.validation.Valid; @@ -35,6 +36,7 @@ public class NotificationTarget extends BaseData implement private TenantId tenantId; @NotBlank @NoXss + @Length(max = 255, message = "cannot be longer than 255 chars") private String name; @NotNull @Valid diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetConfig.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetConfig.java index 2333062f65..231e419fb0 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetConfig.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/targets/NotificationTargetConfig.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import lombok.Data; import org.thingsboard.server.common.data.notification.targets.platform.PlatformUsersNotificationTargetConfig; import org.thingsboard.server.common.data.notification.targets.slack.SlackNotificationTargetConfig; +import org.thingsboard.server.common.data.validation.Length; import org.thingsboard.server.common.data.validation.NoXss; @JsonIgnoreProperties(ignoreUnknown = true) @@ -35,6 +36,7 @@ import org.thingsboard.server.common.data.validation.NoXss; public abstract class NotificationTargetConfig { @NoXss + @Length(max = 500, message = "cannot be longer than 500 chars") private String description; @JsonIgnore diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/EmailDeliveryMethodNotificationTemplate.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/EmailDeliveryMethodNotificationTemplate.java index bf8aca7913..fe909a104e 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/EmailDeliveryMethodNotificationTemplate.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/EmailDeliveryMethodNotificationTemplate.java @@ -21,6 +21,8 @@ import lombok.NoArgsConstructor; import lombok.ToString; import org.apache.commons.lang3.StringUtils; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; +import org.thingsboard.server.common.data.validation.Length; +import org.thingsboard.server.common.data.validation.NoXss; import javax.validation.constraints.NotEmpty; @@ -30,6 +32,8 @@ import javax.validation.constraints.NotEmpty; @ToString(callSuper = true) public class EmailDeliveryMethodNotificationTemplate extends DeliveryMethodNotificationTemplate implements HasSubject { + @NoXss(fieldName = "email subject") + @Length(fieldName = "email subject", max = 250, message = "cannot be longer than 250 chars") @NotEmpty private String subject; diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationTemplate.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationTemplate.java index 2eaff12a9b..71da2f862b 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationTemplate.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationTemplate.java @@ -23,6 +23,7 @@ import org.thingsboard.server.common.data.HasTenantId; import org.thingsboard.server.common.data.id.NotificationTemplateId; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.notification.NotificationType; +import org.thingsboard.server.common.data.validation.Length; import org.thingsboard.server.common.data.validation.NoXss; import javax.validation.Valid; @@ -36,6 +37,7 @@ public class NotificationTemplate extends BaseData imple private TenantId tenantId; @NoXss @NotEmpty + @Length(max = 255, message = "cannot be longer than 255 chars") private String name; @NoXss @NotNull diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationText.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationText.java deleted file mode 100644 index 6cb0e673a5..0000000000 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/NotificationText.java +++ /dev/null @@ -1,30 +0,0 @@ -/** - * Copyright © 2016-2023 The Thingsboard Authors - * - * Licensed 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.thingsboard.server.common.data.notification.template; - -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - -@Data -@AllArgsConstructor -@NoArgsConstructor -public class NotificationText { - - private String body; - private String subject; - -} diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SlackDeliveryMethodNotificationTemplate.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SlackDeliveryMethodNotificationTemplate.java index 735eb3660e..8457677308 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SlackDeliveryMethodNotificationTemplate.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SlackDeliveryMethodNotificationTemplate.java @@ -20,6 +20,7 @@ import lombok.EqualsAndHashCode; import lombok.NoArgsConstructor; import lombok.ToString; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; +import org.thingsboard.server.common.data.validation.NoXss; @Data @NoArgsConstructor @@ -31,6 +32,12 @@ public class SlackDeliveryMethodNotificationTemplate extends DeliveryMethodNotif super(other); } + @NoXss(fieldName = "Slack message") + @Override + public String getBody() { + return super.getBody(); + } + @Override public NotificationDeliveryMethod getMethod() { return NotificationDeliveryMethod.SLACK; diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SmsDeliveryMethodNotificationTemplate.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SmsDeliveryMethodNotificationTemplate.java index 55d2582284..7dc2e494f3 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SmsDeliveryMethodNotificationTemplate.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/SmsDeliveryMethodNotificationTemplate.java @@ -20,6 +20,8 @@ import lombok.EqualsAndHashCode; import lombok.NoArgsConstructor; import lombok.ToString; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; +import org.thingsboard.server.common.data.validation.Length; +import org.thingsboard.server.common.data.validation.NoXss; @Data @NoArgsConstructor @@ -31,6 +33,13 @@ public class SmsDeliveryMethodNotificationTemplate extends DeliveryMethodNotific super(other); } + @NoXss(fieldName = "SMS message") + @Length(fieldName = "SMS message", max = 320, message = "cannot be longer than 320 chars") + @Override + public String getBody() { + return super.getBody(); + } + @Override public NotificationDeliveryMethod getMethod() { return NotificationDeliveryMethod.SMS; diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/WebDeliveryMethodNotificationTemplate.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/WebDeliveryMethodNotificationTemplate.java index ac1e0cfd8c..f26ed00568 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/WebDeliveryMethodNotificationTemplate.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/template/WebDeliveryMethodNotificationTemplate.java @@ -25,6 +25,8 @@ import lombok.NoArgsConstructor; import lombok.ToString; import org.apache.commons.lang3.StringUtils; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; +import org.thingsboard.server.common.data.validation.Length; +import org.thingsboard.server.common.data.validation.NoXss; import javax.validation.constraints.NotEmpty; import java.util.Optional; @@ -35,6 +37,8 @@ import java.util.Optional; @ToString(callSuper = true) public class WebDeliveryMethodNotificationTemplate extends DeliveryMethodNotificationTemplate implements HasSubject { + @NoXss(fieldName = "web notification subject") + @Length(fieldName = "web notification subject", max = 150, message = "cannot be longer than 150 chars") @NotEmpty private String subject; private JsonNode additionalConfig; @@ -45,6 +49,15 @@ public class WebDeliveryMethodNotificationTemplate extends DeliveryMethodNotific this.additionalConfig = other.additionalConfig != null ? other.additionalConfig.deepCopy() : null; } + @NoXss(fieldName = "web notification message") + @Length(fieldName = "web notification message", max = 250, message = "cannot be longer than 250 chars") + @Override + public String getBody() { + return super.getBody(); + } + + @NoXss(fieldName = "web notification button text") + @Length(fieldName = "web notification button text", max = 50, message = "cannot be longer than 50 chars") @JsonIgnore public String getButtonText() { return getButtonConfigProperty("text"); @@ -57,6 +70,8 @@ public class WebDeliveryMethodNotificationTemplate extends DeliveryMethodNotific }); } + @NoXss(fieldName = "web notification button link") + @Length(fieldName = "web notification button link", max = 300, message = "cannot be longer than 300 chars") @JsonIgnore public String getButtonLink() { return getButtonConfigProperty("link"); diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/validation/Length.java b/common/data/src/main/java/org/thingsboard/server/common/data/validation/Length.java index 2b412d0ad7..cb7b9b1693 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/validation/Length.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/validation/Length.java @@ -23,12 +23,12 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.FIELD) +@Target({ElementType.FIELD, ElementType.METHOD}) @Constraint(validatedBy = {}) public @interface Length { String message() default "length must be equal or less than {max}"; - String fieldName(); + String fieldName() default ""; int max() default 255; diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/validation/NoXss.java b/common/data/src/main/java/org/thingsboard/server/common/data/validation/NoXss.java index c99502ed7b..fb7b048f7e 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/validation/NoXss.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/validation/NoXss.java @@ -23,12 +23,16 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.FIELD) +@Target({ElementType.FIELD, ElementType.METHOD}) @Constraint(validatedBy = {}) public @interface NoXss { + String message() default "is malformed"; + String fieldName() default ""; + Class[] groups() default {}; Class[] payload() default {}; + } diff --git a/dao/src/main/java/org/thingsboard/server/dao/service/ConstraintValidator.java b/dao/src/main/java/org/thingsboard/server/dao/service/ConstraintValidator.java index 41a618959a..b884de0094 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/service/ConstraintValidator.java +++ b/dao/src/main/java/org/thingsboard/server/dao/service/ConstraintValidator.java @@ -17,6 +17,7 @@ package org.thingsboard.server.dao.service; import com.google.common.collect.Iterators; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; import org.hibernate.validator.HibernateValidator; import org.hibernate.validator.HibernateValidatorConfiguration; import org.hibernate.validator.cfg.ConstraintMapping; @@ -29,11 +30,13 @@ import org.thingsboard.server.common.data.validation.Length; import org.thingsboard.server.common.data.validation.NoXss; import org.thingsboard.server.dao.exception.DataValidationException; -import javax.validation.Path; +import javax.validation.ConstraintViolation; import javax.validation.Validation; import javax.validation.Validator; import javax.validation.constraints.AssertTrue; -import java.util.List; +import javax.validation.metadata.ConstraintDescriptor; +import java.util.Collection; +import java.util.Set; import java.util.stream.Collectors; @Slf4j @@ -51,26 +54,31 @@ public class ConstraintValidator { } public static void validateFields(Object data, String errorPrefix) { - List constraintsViolations = getConstraintsViolations(data); + Set> constraintsViolations = fieldsValidator.validate(data); if (!constraintsViolations.isEmpty()) { - throw new DataValidationException(errorPrefix + String.join(", ", constraintsViolations)); + throw new DataValidationException(errorPrefix + getErrorMessage(constraintsViolations)); } } - public static List getConstraintsViolations(Object data) { - return fieldsValidator.validate(data).stream() - .map(constraintViolation -> { - String property; - if (constraintViolation.getConstraintDescriptor().getAttributes().containsKey("fieldName")) { - property = constraintViolation.getConstraintDescriptor().getAttributes().get("fieldName").toString(); - } else { - Path propertyPath = constraintViolation.getPropertyPath(); - property = Iterators.getLast(propertyPath.iterator()).toString(); - } - return property + " " + constraintViolation.getMessage(); - }) - .distinct() - .collect(Collectors.toList()); + public static String getErrorMessage(Collection> constraintsViolations) { + return constraintsViolations.stream() + .map(ConstraintValidator::getErrorMessage) + .distinct().sorted().collect(Collectors.joining(", ")); + } + + public static String getErrorMessage(ConstraintViolation constraintViolation) { + ConstraintDescriptor constraintDescriptor = constraintViolation.getConstraintDescriptor(); + String property = (String) constraintDescriptor.getAttributes().get("fieldName"); + if (StringUtils.isEmpty(property) && !(constraintDescriptor.getAnnotation() instanceof AssertTrue)) { + property = Iterators.getLast(constraintViolation.getPropertyPath().iterator()).toString(); + } + + String error = ""; + if (StringUtils.isNotEmpty(property)) { + error += property + " "; + } + error += constraintViolation.getMessage(); + return error; } private static void initializeValidators() { From 542d0dfe0c8899940a826bd9d9be87182731c968 Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Thu, 13 Apr 2023 12:54:44 +0300 Subject: [PATCH 3/5] Don't send notifications to customers for default rules --- .../DefaultNotificationSettingsService.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationSettingsService.java b/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationSettingsService.java index 9438c79b62..8319468c04 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationSettingsService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationSettingsService.java @@ -170,8 +170,7 @@ public class DefaultNotificationSettingsService implements NotificationSettingsS newAlarmRuleTriggerConfig.setAlarmSeverities(null); newAlarmRuleTriggerConfig.setNotifyOn(Set.of(AlarmAction.CREATED)); createRule(tenantId, "New alarm", newAlarmNotificationTemplate.getId(), newAlarmRuleTriggerConfig, - List.of(tenantAdmins.getId(), originatorEntityOwnerUsers.getId()), "Send notification to tenant admins and alarm's customer users " + - "when an alarm is created"); + List.of(tenantAdmins.getId()), "Send notification to tenant admins when an alarm is created"); NotificationTemplate alarmUpdateNotificationTemplate = createTemplate(tenantId, "Alarm update notification", NotificationType.ALARM, "Alarm '${alarmType}' - ${action}", @@ -182,8 +181,7 @@ public class DefaultNotificationSettingsService implements NotificationSettingsS alarmRuleTriggerConfig.setAlarmSeverities(null); alarmRuleTriggerConfig.setNotifyOn(Set.of(AlarmAction.SEVERITY_CHANGED, AlarmAction.ACKNOWLEDGED, AlarmAction.CLEARED)); createRule(tenantId, "Alarm update", alarmUpdateNotificationTemplate.getId(), alarmRuleTriggerConfig, - List.of(tenantAdmins.getId(), originatorEntityOwnerUsers.getId()), "Send notification to tenant admins and alarm's customer users " + - "when any alarm is updated or cleared"); + List.of(tenantAdmins.getId()), "Send notification to tenant admins when any alarm is updated or cleared"); NotificationTemplate deviceActionNotificationTemplate = createTemplate(tenantId, "Device action notification", NotificationType.ENTITY_ACTION, "${entityType} was ${actionType}", @@ -195,8 +193,7 @@ public class DefaultNotificationSettingsService implements NotificationSettingsS deviceActionRuleTriggerConfig.setUpdated(false); deviceActionRuleTriggerConfig.setDeleted(false); createRule(tenantId, "Device created", deviceActionNotificationTemplate.getId(), deviceActionRuleTriggerConfig, - List.of(originatorEntityOwnerUsers.getId()), "Send notification to tenant admins or customer users " + - "when device is created"); + List.of(tenantAdmins.getId()), "Send notification to tenant admins when device is created"); NotificationTemplate deviceActivityNotificationTemplate = createTemplate(tenantId, "Device activity notification", NotificationType.DEVICE_ACTIVITY, "Device '${deviceName}' became ${eventType}", @@ -207,8 +204,7 @@ public class DefaultNotificationSettingsService implements NotificationSettingsS deviceActivityRuleTriggerConfig.setDeviceProfiles(null); deviceActivityRuleTriggerConfig.setNotifyOn(Set.of(DeviceEvent.ACTIVE, DeviceEvent.INACTIVE)); createRule(tenantId, "Device activity status change", deviceActivityNotificationTemplate.getId(), deviceActivityRuleTriggerConfig, - List.of(originatorEntityOwnerUsers.getId()), "Send notification to tenant admins or customer users " + - "when any device changes its activity state"); + List.of(tenantAdmins.getId()), "Send notification to tenant admins when any device changes its activity state"); NotificationTemplate alarmCommentNotificationTemplate = createTemplate(tenantId, "Alarm comment notification", NotificationType.ALARM_COMMENT, "Comment on '${alarmType}' alarm", @@ -221,8 +217,7 @@ public class DefaultNotificationSettingsService implements NotificationSettingsS alarmCommentRuleTriggerConfig.setOnlyUserComments(true); alarmCommentRuleTriggerConfig.setNotifyOnCommentUpdate(false); createRule(tenantId, "Comment on active alarm", alarmCommentNotificationTemplate.getId(), alarmCommentRuleTriggerConfig, - List.of(originatorEntityOwnerUsers.getId()), "Send notification to tenant admins or customer users " + - "when comment is added by user on active alarm"); + List.of(tenantAdmins.getId()), "Send notification to tenant admins when comment is added by user on active alarm"); NotificationTemplate alarmAssignedNotificationTemplate = createTemplate(tenantId, "Alarm assigned notification", NotificationType.ALARM_ASSIGNMENT, "Alarm '${alarmType}' (${alarmSeverity}) was assigned to user", From 7b6f3ed4df034270e3c160888fdbde9c0e144f83 Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Thu, 13 Apr 2023 18:20:37 +0300 Subject: [PATCH 4/5] Notification requests per rule rate limits --- .../limits/DefaultRateLimitService.java | 69 +++++++--- .../service/apiusage/limits/LimitedApi.java | 3 +- .../apiusage/limits/RateLimitService.java | 5 +- .../DefaultNotificationCenter.java | 21 ++- .../NotificationProcessingContext.java | 12 +- .../DefaultNotificationRuleProcessor.java | 10 +- .../DefaultEntitiesExportImportService.java | 4 +- .../src/main/resources/thingsboard.yml | 4 + .../notification/NotificationRuleApiTest.java | 123 +++++++++--------- .../NotificationTemplateApiTest.java | 10 -- .../DefaultTenantProfileConfiguration.java | 1 + ...enant-profile-configuration.component.html | 3 + ...-tenant-profile-configuration.component.ts | 1 + .../tenant/rate-limits/rate-limits.models.ts | 5 +- ui-ngx/src/app/shared/models/tenant.model.ts | 1 + .../assets/locale/locale.constant-en_US.json | 2 + 16 files changed, 165 insertions(+), 109 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/apiusage/limits/DefaultRateLimitService.java b/application/src/main/java/org/thingsboard/server/service/apiusage/limits/DefaultRateLimitService.java index e995a8bd1f..9a0282c840 100644 --- a/application/src/main/java/org/thingsboard/server/service/apiusage/limits/DefaultRateLimitService.java +++ b/application/src/main/java/org/thingsboard/server/service/apiusage/limits/DefaultRateLimitService.java @@ -15,54 +15,81 @@ */ package org.thingsboard.server.service.apiusage.limits; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import lombok.Data; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import org.thingsboard.server.common.data.StringUtils; +import org.thingsboard.server.common.data.id.EntityId; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.msg.tools.TbRateLimits; import org.thingsboard.server.dao.tenant.TbTenantProfileCache; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; +import javax.annotation.PostConstruct; +import java.util.concurrent.TimeUnit; @Service @RequiredArgsConstructor +@Slf4j public class DefaultRateLimitService implements RateLimitService { private final TbTenantProfileCache tenantProfileCache; + @Value("${cache.rateLimits.timeToLiveInMinutes:60}") + private int rateLimitsTtl; + @Value("${cache.rateLimits.maxSize:100000}") + private int rateLimitsCacheMaxSize; - private final Map> rateLimits = new ConcurrentHashMap<>(); + private Cache rateLimits; + + @PostConstruct + private void init() { + rateLimits = Caffeine.newBuilder() + .expireAfterAccess(rateLimitsTtl, TimeUnit.MINUTES) + .maximumSize(rateLimitsCacheMaxSize) + .build(); + } + + @Override + public boolean checkRateLimit(LimitedApi api, TenantId tenantId) { + return checkRateLimit(api, tenantId, tenantId); + } @Override - public boolean checkRateLimit(TenantId tenantId, LimitedApi api) { + public boolean checkRateLimit(LimitedApi api, TenantId tenantId, EntityId entityId) { if (tenantId.isSysTenantId()) { return true; } + RateLimitKey key = new RateLimitKey(api, entityId); + String rateLimitConfig = tenantProfileCache.get(tenantId).getProfileConfiguration() .map(api::getLimitConfig).orElse(null); - - Map rateLimits = this.rateLimits.get(api); if (StringUtils.isEmpty(rateLimitConfig)) { - if (rateLimits != null) { - rateLimits.remove(tenantId); - if (rateLimits.isEmpty()) { - this.rateLimits.remove(api); - } - } + rateLimits.invalidate(key); return true; } + log.trace("[{}] Checking rate limit for {} ({})", entityId, api, rateLimitConfig); - if (rateLimits == null) { - rateLimits = new ConcurrentHashMap<>(); - this.rateLimits.put(api, rateLimits); - } - TbRateLimits rateLimit = rateLimits.get(tenantId); - if (rateLimit == null || !rateLimit.getConfiguration().equals(rateLimitConfig)) { - rateLimit = new TbRateLimits(rateLimitConfig); - rateLimits.put(tenantId, rateLimit); + TbRateLimits rateLimit = rateLimits.asMap().compute(key, (k, limit) -> { + if (limit == null || !limit.getConfiguration().equals(rateLimitConfig)) { + limit = new TbRateLimits(rateLimitConfig); + log.trace("[{}] Created new rate limit bucket for {} ({})", entityId, api, rateLimitConfig); + } + return limit; + }); + boolean success = rateLimit.tryConsume(); + if (!success) { + log.debug("[{}] Rate limit exceeded for {} ({})", entityId, api, rateLimitConfig); } + return success; + } - return rateLimit.tryConsume(); + @Data(staticConstructor = "of") + private static class RateLimitKey { + private final LimitedApi api; + private final EntityId entityId; } } diff --git a/application/src/main/java/org/thingsboard/server/service/apiusage/limits/LimitedApi.java b/application/src/main/java/org/thingsboard/server/service/apiusage/limits/LimitedApi.java index f69ece6661..4f216ffaca 100644 --- a/application/src/main/java/org/thingsboard/server/service/apiusage/limits/LimitedApi.java +++ b/application/src/main/java/org/thingsboard/server/service/apiusage/limits/LimitedApi.java @@ -25,7 +25,8 @@ public enum LimitedApi { ENTITY_EXPORT(DefaultTenantProfileConfiguration::getTenantEntityExportRateLimit), ENTITY_IMPORT(DefaultTenantProfileConfiguration::getTenantEntityImportRateLimit), - NOTIFICATION_REQUEST(DefaultTenantProfileConfiguration::getTenantNotificationRequestsRateLimit); + NOTIFICATION_REQUESTS(DefaultTenantProfileConfiguration::getTenantNotificationRequestsRateLimit), + NOTIFICATION_REQUESTS_PER_RULE(DefaultTenantProfileConfiguration::getTenantNotificationRequestsPerRuleRateLimit); private final Function configExtractor; diff --git a/application/src/main/java/org/thingsboard/server/service/apiusage/limits/RateLimitService.java b/application/src/main/java/org/thingsboard/server/service/apiusage/limits/RateLimitService.java index c984ec8fa5..3fde98618e 100644 --- a/application/src/main/java/org/thingsboard/server/service/apiusage/limits/RateLimitService.java +++ b/application/src/main/java/org/thingsboard/server/service/apiusage/limits/RateLimitService.java @@ -15,10 +15,13 @@ */ package org.thingsboard.server.service.apiusage.limits; +import org.thingsboard.server.common.data.id.EntityId; import org.thingsboard.server.common.data.id.TenantId; public interface RateLimitService { - boolean checkRateLimit(TenantId tenantId, LimitedApi api); + boolean checkRateLimit(LimitedApi api, TenantId tenantId); + + boolean checkRateLimit(LimitedApi api, TenantId tenantId, EntityId entityId); } 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 d297f70a7f..91bc0863db 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 @@ -107,8 +107,10 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple @Override public NotificationRequest processNotificationRequest(TenantId tenantId, NotificationRequest request, Consumer callback) { - if (!rateLimitService.checkRateLimit(tenantId, LimitedApi.NOTIFICATION_REQUEST)) { - throw new TbRateLimitsException(EntityType.TENANT); + if (request.getRuleId() == null) { + if (!rateLimitService.checkRateLimit(LimitedApi.NOTIFICATION_REQUESTS, tenantId)) { + throw new TbRateLimitsException(EntityType.TENANT); + } } NotificationTemplate notificationTemplate; @@ -119,8 +121,10 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple } if (notificationTemplate == null) throw new IllegalArgumentException("Template is missing"); + Set deliveryMethods = new HashSet<>(); List targets = request.getTargets().stream().map(NotificationTargetId::new) - .map(id -> notificationTargetService.findNotificationTargetById(tenantId, id)).collect(Collectors.toList()); + .map(id -> notificationTargetService.findNotificationTargetById(tenantId, id)) + .collect(Collectors.toList()); NotificationRuleId ruleId = request.getRuleId(); notificationTemplate.getConfiguration().getDeliveryMethodsTemplates().forEach((deliveryMethod, template) -> { @@ -128,14 +132,22 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple try { channels.get(deliveryMethod).check(tenantId); } catch (Exception e) { - throw new IllegalArgumentException(e.getMessage()); + if (ruleId == null) { + throw new IllegalArgumentException(e.getMessage()); + } else { + return; // if originated by rule - just ignore delivery method + } } if (ruleId == null) { if (targets.stream().noneMatch(target -> target.getConfiguration().getType().getSupportedDeliveryMethods().contains(deliveryMethod))) { throw new IllegalArgumentException("Recipients for " + deliveryMethod.getName() + " delivery method not chosen"); } } + deliveryMethods.add(deliveryMethod); }); + if (deliveryMethods.isEmpty()) { + throw new IllegalArgumentException("No delivery methods to send notification with"); + } if (request.getAdditionalConfig() != null) { NotificationRequestConfig config = request.getAdditionalConfig(); @@ -155,6 +167,7 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple NotificationProcessingContext ctx = NotificationProcessingContext.builder() .tenantId(tenantId) .request(request) + .deliveryMethods(deliveryMethods) .template(notificationTemplate) .settings(settings) .build(); diff --git a/application/src/main/java/org/thingsboard/server/service/notification/NotificationProcessingContext.java b/application/src/main/java/org/thingsboard/server/service/notification/NotificationProcessingContext.java index 8f9b7d048b..25ff1dd50e 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/NotificationProcessingContext.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/NotificationProcessingContext.java @@ -48,20 +48,21 @@ public class NotificationProcessingContext { private final NotificationSettings settings; @Getter private final NotificationRequest request; - + @Getter + private final Set deliveryMethods; @Getter private final NotificationTemplate notificationTemplate; + private final Map templates; @Getter - private Set deliveryMethods; - @Getter private final NotificationRequestStats stats; - @Builder - public NotificationProcessingContext(TenantId tenantId, NotificationRequest request, NotificationTemplate template, NotificationSettings settings) { + public NotificationProcessingContext(TenantId tenantId, NotificationRequest request, Set deliveryMethods, + NotificationTemplate template, NotificationSettings settings) { this.tenantId = tenantId; this.request = request; + this.deliveryMethods = deliveryMethods; this.settings = settings; this.notificationTemplate = template; this.templates = new EnumMap<>(NotificationDeliveryMethod.class); @@ -77,7 +78,6 @@ public class NotificationProcessingContext { templates.put(deliveryMethod, template); } }); - deliveryMethods = templates.keySet(); } public C getDeliveryMethodConfig(NotificationDeliveryMethod deliveryMethod) { diff --git a/application/src/main/java/org/thingsboard/server/service/notification/rule/DefaultNotificationRuleProcessor.java b/application/src/main/java/org/thingsboard/server/service/notification/rule/DefaultNotificationRuleProcessor.java index 2cb4c37210..8b3edbb812 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/rule/DefaultNotificationRuleProcessor.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/rule/DefaultNotificationRuleProcessor.java @@ -41,9 +41,11 @@ import org.thingsboard.server.common.msg.notification.trigger.RuleEngineMsgTrigg import org.thingsboard.server.common.msg.plugin.ComponentLifecycleMsg; import org.thingsboard.server.common.msg.queue.ServiceType; import org.thingsboard.server.dao.notification.NotificationRequestService; -import org.thingsboard.server.service.notification.rule.cache.NotificationRulesCache; import org.thingsboard.server.queue.discovery.PartitionService; +import org.thingsboard.server.service.apiusage.limits.LimitedApi; +import org.thingsboard.server.service.apiusage.limits.RateLimitService; import org.thingsboard.server.service.executors.NotificationExecutorService; +import org.thingsboard.server.service.notification.rule.cache.NotificationRulesCache; import org.thingsboard.server.service.notification.rule.trigger.NotificationRuleTriggerProcessor; import org.thingsboard.server.service.notification.rule.trigger.RuleEngineMsgNotificationRuleTriggerProcessor; @@ -65,6 +67,7 @@ public class DefaultNotificationRuleProcessor implements NotificationRuleProcess private final NotificationRulesCache notificationRulesCache; private final NotificationRequestService notificationRequestService; private final PartitionService partitionService; + private final RateLimitService rateLimitService; @Autowired @Lazy private NotificationCenter notificationCenter; private final NotificationExecutorService notificationExecutor; @@ -119,6 +122,11 @@ public class DefaultNotificationRuleProcessor implements NotificationRuleProcess } if (matchesFilter(trigger, triggerConfig)) { + if (!rateLimitService.checkRateLimit(LimitedApi.NOTIFICATION_REQUESTS_PER_RULE, rule.getTenantId(), rule.getId())) { + log.debug("[{}] Rate limit for notification requests per rule was exceeded (rule '{}')", rule.getTenantId(), rule.getName()); + return; + } + NotificationInfo notificationInfo = constructNotificationInfo(trigger, triggerConfig); rule.getRecipientsConfig().getTargetsTable().forEach((delay, targets) -> { submitNotificationRequest(targets, rule, trigger.getOriginatorEntityId(), notificationInfo, delay); diff --git a/application/src/main/java/org/thingsboard/server/service/sync/ie/DefaultEntitiesExportImportService.java b/application/src/main/java/org/thingsboard/server/service/sync/ie/DefaultEntitiesExportImportService.java index b646cb7ef6..793c25d625 100644 --- a/application/src/main/java/org/thingsboard/server/service/sync/ie/DefaultEntitiesExportImportService.java +++ b/application/src/main/java/org/thingsboard/server/service/sync/ie/DefaultEntitiesExportImportService.java @@ -73,7 +73,7 @@ public class DefaultEntitiesExportImportService implements EntitiesExportImportS @Override public , I extends EntityId> EntityExportData exportEntity(EntitiesExportCtx ctx, I entityId) throws ThingsboardException { - if (!rateLimitService.checkRateLimit(ctx.getTenantId(), LimitedApi.ENTITY_EXPORT)) { + if (!rateLimitService.checkRateLimit(LimitedApi.ENTITY_EXPORT, ctx.getTenantId())) { throw new ThingsboardException("Rate limit for entities export is exceeded", ThingsboardErrorCode.TOO_MANY_REQUESTS); } @@ -85,7 +85,7 @@ public class DefaultEntitiesExportImportService implements EntitiesExportImportS @Override public , I extends EntityId> EntityImportResult importEntity(EntitiesImportCtx ctx, EntityExportData exportData) throws ThingsboardException { - if (!rateLimitService.checkRateLimit(ctx.getTenantId(), LimitedApi.ENTITY_IMPORT)) { + if (!rateLimitService.checkRateLimit(LimitedApi.ENTITY_IMPORT, ctx.getTenantId())) { throw new ThingsboardException("Rate limit for entities import is exceeded", ThingsboardErrorCode.TOO_MANY_REQUESTS); } if (exportData.getEntity() == null || exportData.getEntity().getId() == null) { diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index d51a2f228f..dea1756ae0 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -478,10 +478,14 @@ cache: entityCount: timeToLiveInMinutes: "${CACHE_SPECS_ENTITY_COUNT_TTL:1440}" maxSize: "${CACHE_SPECS_ENTITY_COUNT_MAX_SIZE:100000}" + # deliberately placed outside 'specs' group above notificationRules: timeToLiveInMinutes: "${CACHE_SPECS_NOTIFICATION_RULES_TTL:30}" maxSize: "${CACHE_SPECS_NOTIFICATION_RULES_MAX_SIZE:1000}" + rateLimits: + timeToLiveInMinutes: "${CACHE_SPECS_RATE_LIMITS_TTL:60}" + maxSize: "${CACHE_SPECS_RATE_LIMITS_MAX_SIZE:100000}" #Disable this because it is not required. spring.data.redis.repositories.enabled: false diff --git a/application/src/test/java/org/thingsboard/server/service/notification/NotificationRuleApiTest.java b/application/src/test/java/org/thingsboard/server/service/notification/NotificationRuleApiTest.java index 3e12cb6d05..8fec807d45 100644 --- a/application/src/test/java/org/thingsboard/server/service/notification/NotificationRuleApiTest.java +++ b/application/src/test/java/org/thingsboard/server/service/notification/NotificationRuleApiTest.java @@ -23,14 +23,12 @@ import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.data.util.Pair; -import org.springframework.test.context.TestPropertySource; import org.thingsboard.common.util.JacksonUtil; -import org.thingsboard.rule.engine.debug.TbMsgGeneratorNode; -import org.thingsboard.rule.engine.debug.TbMsgGeneratorNodeConfiguration; import org.thingsboard.server.common.data.DataConstants; import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.DeviceProfile; import org.thingsboard.server.common.data.EntityType; +import org.thingsboard.server.common.data.TenantProfile; import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.alarm.Alarm; import org.thingsboard.server.common.data.alarm.AlarmSearchStatus; @@ -43,8 +41,7 @@ import org.thingsboard.server.common.data.device.profile.AlarmConditionKeyType; import org.thingsboard.server.common.data.device.profile.AlarmRule; import org.thingsboard.server.common.data.device.profile.DeviceProfileAlarm; import org.thingsboard.server.common.data.device.profile.SimpleAlarmConditionSpec; -import org.thingsboard.server.common.data.id.NotificationRuleId; -import org.thingsboard.server.common.data.id.RuleChainId; +import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.notification.Notification; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; import org.thingsboard.server.common.data.notification.NotificationRequest; @@ -59,25 +56,21 @@ import org.thingsboard.server.common.data.notification.rule.trigger.AlarmNotific import org.thingsboard.server.common.data.notification.rule.trigger.AlarmNotificationRuleTriggerConfig.AlarmAction; import org.thingsboard.server.common.data.notification.rule.trigger.EntityActionNotificationRuleTriggerConfig; import org.thingsboard.server.common.data.notification.rule.trigger.NotificationRuleTriggerType; -import org.thingsboard.server.common.data.notification.rule.trigger.RuleEngineComponentLifecycleEventNotificationRuleTriggerConfig; import org.thingsboard.server.common.data.notification.targets.NotificationTarget; import org.thingsboard.server.common.data.notification.template.NotificationTemplate; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; -import org.thingsboard.server.common.data.plugin.ComponentLifecycleEvent; import org.thingsboard.server.common.data.query.BooleanFilterPredicate; import org.thingsboard.server.common.data.query.EntityKeyValueType; import org.thingsboard.server.common.data.query.FilterPredicateValue; -import org.thingsboard.server.common.data.rule.RuleChain; -import org.thingsboard.server.common.data.rule.RuleChainMetaData; -import org.thingsboard.server.common.data.rule.RuleNode; -import org.thingsboard.server.common.data.script.ScriptLanguage; import org.thingsboard.server.common.data.security.Authority; -import org.thingsboard.server.dao.alarm.AlarmService; +import org.thingsboard.server.common.data.tenant.profile.DefaultTenantProfileConfiguration; +import org.thingsboard.server.common.data.tenant.profile.TenantProfileData; import org.thingsboard.server.dao.notification.NotificationRequestService; -import org.thingsboard.server.dao.notification.NotificationRuleService; -import org.thingsboard.server.dao.notification.NotificationTemplateService; import org.thingsboard.server.dao.service.DaoSqlTest; +import org.thingsboard.server.dao.tenant.TenantProfileService; +import org.thingsboard.server.service.apiusage.limits.LimitedApi; +import org.thingsboard.server.service.apiusage.limits.RateLimitService; import org.thingsboard.server.service.telemetry.AlarmSubscriptionService; import java.util.ArrayList; @@ -96,9 +89,6 @@ import static org.awaitility.Awaitility.await; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @DaoSqlTest -@TestPropertySource(properties = { - "js.evaluator=local" -}) public class NotificationRuleApiTest extends AbstractNotificationApiTest { @SpyBean @@ -106,18 +96,13 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest { @Autowired private NotificationRequestService notificationRequestService; @Autowired - private NotificationRuleService notificationRuleService; + private TenantProfileService tenantProfileService; @Autowired - private NotificationTemplateService notificationTemplateService; - - @SpyBean - private AlarmService alarmService; + private RateLimitService rateLimitService; @Before public void beforeEach() throws Exception { loginTenantAdmin(); - notificationRuleService.deleteNotificationRulesByTenantId(tenantId); - notificationTemplateService.deleteNotificationTemplatesByTenantId(tenantId); } @Test @@ -208,7 +193,7 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest { String alarmType = "myBoolIsTrue"; - DeviceProfile deviceProfile = createDeviceProfileWithAlarmRules(notificationRule.getId(), alarmType); + DeviceProfile deviceProfile = createDeviceProfileWithAlarmRules(alarmType); Device device = createDevice("Device 1", deviceProfile.getName(), "1234"); clients.values().forEach(wsClient -> { @@ -272,7 +257,7 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest { notificationRule.setTriggerType(NotificationRuleTriggerType.ALARM); String alarmType = "myBoolIsTrue"; - DeviceProfile deviceProfile = createDeviceProfileWithAlarmRules(notificationRule.getId(), alarmType); + DeviceProfile deviceProfile = createDeviceProfileWithAlarmRules(alarmType); Device device = createDevice("Device 1", deviceProfile.getName(), "1234"); AlarmNotificationRuleTriggerConfig triggerConfig = new AlarmNotificationRuleTriggerConfig(); @@ -352,7 +337,56 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest { assertThat(ruleInfo.getDeliveryMethods()).containsOnly(deliveryMethods); } - private DeviceProfile createDeviceProfileWithAlarmRules(NotificationRuleId notificationRuleId, String alarmType) { + @Test + public void testNotificationRequestsPerRuleRateLimits() throws Exception { + int notificationRequestsLimit = 10; + TenantProfile tenantProfile = tenantProfileService.findDefaultTenantProfile(TenantId.SYS_TENANT_ID); + TenantProfileData profileData = tenantProfile.getProfileData(); + DefaultTenantProfileConfiguration profileConfiguration = (DefaultTenantProfileConfiguration) profileData.getConfiguration(); + profileConfiguration.setTenantNotificationRequestsPerRuleRateLimit(notificationRequestsLimit + ":300"); + tenantProfile.setProfileData(profileData); + loginSysAdmin(); + doPost("/api/tenantProfile", tenantProfile).andExpect(status().isOk()); + loginTenantAdmin(); + + NotificationRule rule = new NotificationRule(); + rule.setName("Device created"); + rule.setTriggerType(NotificationRuleTriggerType.ENTITY_ACTION); + NotificationTemplate template = createNotificationTemplate(NotificationType.ENTITY_ACTION, "Device created", "Device created", + NotificationDeliveryMethod.WEB, NotificationDeliveryMethod.SMS); + rule.setTemplateId(template.getId()); + EntityActionNotificationRuleTriggerConfig triggerConfig = new EntityActionNotificationRuleTriggerConfig(); + triggerConfig.setEntityTypes(Set.of(EntityType.DEVICE)); + triggerConfig.setCreated(true); + rule.setTriggerConfig(triggerConfig); + NotificationTarget target = createNotificationTarget(tenantAdminUserId); + DefaultNotificationRuleRecipientsConfig recipientsConfig = new DefaultNotificationRuleRecipientsConfig(); + recipientsConfig.setTriggerType(NotificationRuleTriggerType.ENTITY_ACTION); + recipientsConfig.setTargets(List.of(target.getUuidId())); + rule.setRecipientsConfig(recipientsConfig); + rule = saveNotificationRule(rule); + + for (int i = 0; i < notificationRequestsLimit; i++) { + String name = "device " + i; + createDevice(name, name); + } + await().atMost(5, TimeUnit.SECONDS) + .untilAsserted(() -> { + assertThat(getMyNotifications(false, 100)).size().isEqualTo(notificationRequestsLimit); + }); + for (int i = 0; i < 5; i++) { + String name = "device " + (notificationRequestsLimit + i); + createDevice(name, name); + } + + boolean rateLimitExceeded = !rateLimitService.checkRateLimit(LimitedApi.NOTIFICATION_REQUESTS_PER_RULE, tenantId, rule.getId()); + assertThat(rateLimitExceeded).isTrue(); + + TimeUnit.SECONDS.sleep(3); + assertThat(getMyNotifications(false, 100)).size().isEqualTo(notificationRequestsLimit); + } + + private DeviceProfile createDeviceProfileWithAlarmRules(String alarmType) { DeviceProfile deviceProfile = createDeviceProfile("For notification rule test"); deviceProfile.setTenantId(tenantId); @@ -387,41 +421,6 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest { return deviceProfile; } - private RuleChain createEmptyRuleChain(String name) { - RuleChain ruleChain = new RuleChain(); - ruleChain.setName(name); - ruleChain.setTenantId(tenantId); - ruleChain.setRoot(false); - ruleChain.setDebugMode(false); - ruleChain = doPost("/api/ruleChain", ruleChain, RuleChain.class); - - RuleChainMetaData metaData = new RuleChainMetaData(); - metaData.setRuleChainId(ruleChain.getId()); - metaData.setNodes(List.of()); - metaData = doPost("/api/ruleChain/metadata", metaData, RuleChainMetaData.class); - return ruleChain; - } - - private RuleNode addRuleNodeWithError(RuleChainId ruleChainId, String name) { - RuleChainMetaData metaData = new RuleChainMetaData(); - metaData.setRuleChainId(ruleChainId); - - RuleNode generatorNodeWithError = new RuleNode(); - generatorNodeWithError.setName(name); - generatorNodeWithError.setType(TbMsgGeneratorNode.class.getName()); - TbMsgGeneratorNodeConfiguration generatorNodeConfiguration = new TbMsgGeneratorNodeConfiguration(); - generatorNodeConfiguration.setScriptLang(ScriptLanguage.JS); - generatorNodeConfiguration.setPeriodInSeconds(1000); - generatorNodeConfiguration.setMsgCount(1); - generatorNodeConfiguration.setJsScript("[return"); - generatorNodeWithError.setConfiguration(mapper.valueToTree(generatorNodeConfiguration)); - - metaData.setNodes(List.of(generatorNodeWithError)); - metaData.setFirstNodeIndex(0); - metaData = doPost("/api/ruleChain/metadata", metaData, RuleChainMetaData.class); - return metaData.getNodes().get(0); - } - private NotificationRule saveNotificationRule(NotificationRule notificationRule) { return doPost("/api/notification/rule", notificationRule, NotificationRule.class); } diff --git a/application/src/test/java/org/thingsboard/server/service/notification/NotificationTemplateApiTest.java b/application/src/test/java/org/thingsboard/server/service/notification/NotificationTemplateApiTest.java index 1b843d61b4..ce42298d9f 100644 --- a/application/src/test/java/org/thingsboard/server/service/notification/NotificationTemplateApiTest.java +++ b/application/src/test/java/org/thingsboard/server/service/notification/NotificationTemplateApiTest.java @@ -19,7 +19,6 @@ import com.fasterxml.jackson.core.type.TypeReference; import org.apache.commons.lang3.StringUtils; import org.junit.Before; import org.junit.Test; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.ResultMatcher; import org.thingsboard.server.common.data.id.IdBased; @@ -30,8 +29,6 @@ import org.thingsboard.server.common.data.notification.template.NotificationTemp import org.thingsboard.server.common.data.notification.template.NotificationTemplateConfig; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; -import org.thingsboard.server.dao.notification.NotificationRuleService; -import org.thingsboard.server.dao.notification.NotificationTemplateService; import org.thingsboard.server.dao.service.DaoSqlTest; import java.util.List; @@ -43,16 +40,9 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. @DaoSqlTest public class NotificationTemplateApiTest extends AbstractNotificationApiTest { - @Autowired - private NotificationTemplateService templateService; - @Autowired - private NotificationRuleService notificationRuleService; - @Before public void beforeEach() throws Exception { loginTenantAdmin(); - notificationRuleService.deleteNotificationRulesByTenantId(tenantId); - templateService.deleteNotificationTemplatesByTenantId(tenantId); } @Test diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/tenant/profile/DefaultTenantProfileConfiguration.java b/common/data/src/main/java/org/thingsboard/server/common/data/tenant/profile/DefaultTenantProfileConfiguration.java index 030587f50f..66fba5f832 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/tenant/profile/DefaultTenantProfileConfiguration.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/tenant/profile/DefaultTenantProfileConfiguration.java @@ -48,6 +48,7 @@ public class DefaultTenantProfileConfiguration implements TenantProfileConfigura private String tenantEntityExportRateLimit; private String tenantEntityImportRateLimit; private String tenantNotificationRequestsRateLimit; + private String tenantNotificationRequestsPerRuleRateLimit; private long maxTransportMessages; private long maxTransportDataPoints; diff --git a/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.html b/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.html index 81a1206800..7c0ce98e4a 100644 --- a/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.html +++ b/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.html @@ -480,6 +480,9 @@ + + diff --git a/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.ts b/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.ts index e070d760ad..cbb5c7444e 100644 --- a/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.ts +++ b/ui-ngx/src/app/modules/home/components/profile/tenant/default-tenant-profile-configuration.component.ts @@ -73,6 +73,7 @@ export class DefaultTenantProfileConfigurationComponent implements ControlValueA tenantEntityExportRateLimit: [null, []], tenantEntityImportRateLimit: [null, []], tenantNotificationRequestsRateLimit: [null, []], + tenantNotificationRequestsPerRuleRateLimit: [null, []], maxTransportMessages: [null, [Validators.required, Validators.min(0)]], maxTransportDataPoints: [null, [Validators.required, Validators.min(0)]], maxREExecutions: [null, [Validators.required, Validators.min(0)]], diff --git a/ui-ngx/src/app/modules/home/components/profile/tenant/rate-limits/rate-limits.models.ts b/ui-ngx/src/app/modules/home/components/profile/tenant/rate-limits/rate-limits.models.ts index 09ae3f5139..26326e9f63 100644 --- a/ui-ngx/src/app/modules/home/components/profile/tenant/rate-limits/rate-limits.models.ts +++ b/ui-ngx/src/app/modules/home/components/profile/tenant/rate-limits/rate-limits.models.ts @@ -34,7 +34,8 @@ export enum RateLimitsType { CASSANDRA_QUERY_TENANT_RATE_LIMITS_CONFIGURATION = 'CASSANDRA_QUERY_TENANT_RATE_LIMITS_CONFIGURATION', TENANT_ENTITY_EXPORT_RATE_LIMIT = 'TENANT_ENTITY_EXPORT_RATE_LIMIT', TENANT_ENTITY_IMPORT_RATE_LIMIT = 'TENANT_ENTITY_IMPORT_RATE_LIMIT', - TENANT_NOTIFICATION_REQUEST_RATE_LIMIT = 'TENANT_NOTIFICATION_REQUEST_RATE_LIMIT' + TENANT_NOTIFICATION_REQUEST_RATE_LIMIT = 'TENANT_NOTIFICATION_REQUEST_RATE_LIMIT', + TENANT_NOTIFICATION_REQUESTS_PER_RULE_RATE_LIMIT = 'TENANT_NOTIFICATION_REQUESTS_PER_RULE_RATE_LIMIT' } export const rateLimitsLabelTranslationMap = new Map( @@ -52,6 +53,7 @@ export const rateLimitsLabelTranslationMap = new Map( [RateLimitsType.TENANT_ENTITY_EXPORT_RATE_LIMIT, 'tenant-profile.tenant-entity-export-rate-limit'], [RateLimitsType.TENANT_ENTITY_IMPORT_RATE_LIMIT, 'tenant-profile.tenant-entity-import-rate-limit'], [RateLimitsType.TENANT_NOTIFICATION_REQUEST_RATE_LIMIT, 'tenant-profile.tenant-notification-request-rate-limit'], + [RateLimitsType.TENANT_NOTIFICATION_REQUESTS_PER_RULE_RATE_LIMIT, 'tenant-profile.tenant-notification-requests-per-rule-rate-limit'], ] ); @@ -70,6 +72,7 @@ export const rateLimitsDialogTitleTranslationMap = new Map Date: Thu, 13 Apr 2023 18:41:28 +0300 Subject: [PATCH 5/5] Fix getNotificationRequestPreview --- .../thingsboard/server/controller/NotificationController.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/application/src/main/java/org/thingsboard/server/controller/NotificationController.java b/application/src/main/java/org/thingsboard/server/controller/NotificationController.java index d36eb9913d..23aaba0e48 100644 --- a/application/src/main/java/org/thingsboard/server/controller/NotificationController.java +++ b/application/src/main/java/org/thingsboard/server/controller/NotificationController.java @@ -251,9 +251,12 @@ public class NotificationController extends BaseController { preview.setRecipientsCountByTarget(recipientsCountByTarget); preview.setTotalRecipientsCount(recipientsCountByTarget.values().stream().mapToInt(Integer::intValue).sum()); + Set deliveryMethods = template.getConfiguration().getDeliveryMethodsTemplates().entrySet() + .stream().filter(entry -> entry.getValue().isEnabled()).map(Map.Entry::getKey).collect(Collectors.toSet()); NotificationProcessingContext ctx = NotificationProcessingContext.builder() .tenantId(user.getTenantId()) .request(request) + .deliveryMethods(deliveryMethods) .template(template) .settings(null) .build();