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 8f6a6dc177..b379957e0f 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 @@ -238,10 +238,13 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple private void processForRecipient(NotificationDeliveryMethod deliveryMethod, NotificationRecipient recipient, NotificationProcessingContext ctx) throws Exception { if (ctx.getStats().contains(deliveryMethod, recipient.getId())) { throw new AlreadySentException(); + } else { + ctx.getStats().reportProcessed(deliveryMethod, recipient.getId()); } + if (recipient instanceof User && ctx.getRequest().getRuleId() != null) { UserNotificationSettings settings = notificationSettingsService.getUserNotificationSettings(ctx.getTenantId(), (User) recipient, false); - Set enabledDeliveryMethods = settings.getEnabledDeliveryMethods(ctx.getRequest().getRuleId()); + Set enabledDeliveryMethods = settings.getEnabledDeliveryMethods(ctx.getNotificationType()); if (!enabledDeliveryMethods.contains(deliveryMethod)) { throw new RuntimeException("User disabled " + deliveryMethod.getName() + " notifications of this type"); } @@ -260,7 +263,7 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple Notification notification = Notification.builder() .requestId(request.getId()) .recipientId(recipient.getId()) - .type(ctx.getNotificationTemplate().getNotificationType()) + .type(ctx.getNotificationType()) .subject(processedTemplate.getSubject()) .text(processedTemplate.getBody()) .additionalConfig(processedTemplate.getAdditionalConfig()) 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 27a9cabe43..c4d8895266 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 @@ -23,6 +23,7 @@ import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; import org.thingsboard.server.common.data.notification.NotificationRequest; import org.thingsboard.server.common.data.notification.NotificationRequestStats; +import org.thingsboard.server.common.data.notification.NotificationType; import org.thingsboard.server.common.data.notification.settings.NotificationDeliveryMethodConfig; import org.thingsboard.server.common.data.notification.settings.NotificationSettings; import org.thingsboard.server.common.data.notification.targets.NotificationRecipient; @@ -52,6 +53,8 @@ public class NotificationProcessingContext { private final Set deliveryMethods; @Getter private final NotificationTemplate notificationTemplate; + @Getter + private final NotificationType notificationType; private final Map templates; @Getter @@ -65,6 +68,7 @@ public class NotificationProcessingContext { this.deliveryMethods = deliveryMethods; this.settings = settings; this.notificationTemplate = template; + this.notificationType = template.getNotificationType(); this.templates = new EnumMap<>(NotificationDeliveryMethod.class); this.stats = new NotificationRequestStats(); init(); 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 851550eee7..89b77fcf99 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 @@ -15,6 +15,7 @@ */ package org.thingsboard.server.service.notification; +import com.google.common.util.concurrent.SettableFuture; import lombok.extern.slf4j.Slf4j; import org.assertj.core.data.Offset; import org.java_websocket.client.WebSocketClient; @@ -23,6 +24,7 @@ import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.thingsboard.rule.engine.api.NotificationCenter; import org.thingsboard.server.common.data.User; +import org.thingsboard.server.common.data.id.NotificationRuleId; import org.thingsboard.server.common.data.id.NotificationTargetId; import org.thingsboard.server.common.data.notification.Notification; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; @@ -35,6 +37,7 @@ import org.thingsboard.server.common.data.notification.NotificationRequestStatus import org.thingsboard.server.common.data.notification.NotificationType; import org.thingsboard.server.common.data.notification.settings.NotificationSettings; import org.thingsboard.server.common.data.notification.settings.SlackNotificationDeliveryMethodConfig; +import org.thingsboard.server.common.data.notification.settings.UserNotificationSettings; import org.thingsboard.server.common.data.notification.targets.NotificationTarget; import org.thingsboard.server.common.data.notification.targets.platform.CustomerUsersFilter; import org.thingsboard.server.common.data.notification.targets.platform.PlatformUsersNotificationTargetConfig; @@ -59,6 +62,8 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.UUID; import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; @@ -470,6 +475,54 @@ public class NotificationApiTest extends AbstractNotificationApiTest { assertThat(stats.getSent().get(NotificationDeliveryMethod.WEB)).hasValue(1); } + @Test + public void testUserNotificationSettings() throws Exception { + var entityActionNotificationPref = new UserNotificationSettings.NotificationPref(); + entityActionNotificationPref.setEnabled(true); + entityActionNotificationPref.setEnabledDeliveryMethods(Set.of(NotificationDeliveryMethod.WEB)); + + var entitiesLimitNotificationPref = new UserNotificationSettings.NotificationPref(); + entitiesLimitNotificationPref.setEnabled(true); + entitiesLimitNotificationPref.setEnabledDeliveryMethods(Set.of(NotificationDeliveryMethod.SMS)); + + var apiUsageLimitNotificationPref = new UserNotificationSettings.NotificationPref(); + apiUsageLimitNotificationPref.setEnabled(false); + apiUsageLimitNotificationPref.setEnabledDeliveryMethods(Set.of(NotificationDeliveryMethod.WEB)); + + UserNotificationSettings settings = new UserNotificationSettings(Map.of( + NotificationType.ENTITY_ACTION, entityActionNotificationPref, + NotificationType.ENTITIES_LIMIT, entitiesLimitNotificationPref, + NotificationType.API_USAGE_LIMIT, apiUsageLimitNotificationPref + )); + doPost("/api/notification/settings/user", settings, UserNotificationSettings.class); + + var entityActionNotificationTemplate = createNotificationTemplate(NotificationType.ENTITY_ACTION, "Entity action", "Entity action", NotificationDeliveryMethod.WEB); + var entitiesLimitNotificationTemplate = createNotificationTemplate(NotificationType.ENTITIES_LIMIT, "Entities limit", "Entities limit", NotificationDeliveryMethod.WEB); + var apiUsageLimitNotificationTemplate = createNotificationTemplate(NotificationType.API_USAGE_LIMIT, "API usage limit", "API usage limit", NotificationDeliveryMethod.WEB); + NotificationTarget target = createNotificationTarget(tenantAdminUserId); + + NotificationRequest notificationRequest = NotificationRequest.builder() + .tenantId(tenantId) + .templateId(entityActionNotificationTemplate.getId()) + .originatorEntityId(tenantAdminUserId) + .targets(List.of(target.getUuidId())) + .ruleId(new NotificationRuleId(UUID.randomUUID())) // to trigger user settings check + .build(); + NotificationRequestStats stats = submitNotificationRequestAndWait(notificationRequest); + assertThat(stats.getErrors()).isEmpty(); + assertThat(stats.getSent().get(NotificationDeliveryMethod.WEB).get()).isOne(); + + notificationRequest.setTemplateId(entitiesLimitNotificationTemplate.getId()); + stats = submitNotificationRequestAndWait(notificationRequest); + assertThat(stats.getSent().get(NotificationDeliveryMethod.WEB)).matches(n -> n == null || n.get() == 0); + assertThat(stats.getErrors().get(NotificationDeliveryMethod.WEB).values()).first().asString().contains("disabled"); + + notificationRequest.setTemplateId(apiUsageLimitNotificationTemplate.getId()); + stats = submitNotificationRequestAndWait(notificationRequest); + assertThat(stats.getSent().get(NotificationDeliveryMethod.WEB)).matches(n -> n == null || n.get() == 0); + assertThat(stats.getErrors().get(NotificationDeliveryMethod.WEB).values()).first().asString().contains("disabled"); + } + @Test public void testSlackNotifications() throws Exception { NotificationSettings settings = new NotificationSettings(); @@ -524,6 +577,12 @@ public class NotificationApiTest extends AbstractNotificationApiTest { assertThat(stats.getErrors().get(NotificationDeliveryMethod.SLACK).values()).containsExactly(errorMessage); } + private NotificationRequestStats submitNotificationRequestAndWait(NotificationRequest notificationRequest) throws Exception { + SettableFuture future = SettableFuture.create(); + notificationCenter.processNotificationRequest(notificationRequest.getTenantId(), notificationRequest, future::set); + return future.get(30, TimeUnit.SECONDS); + } + private void checkFullNotificationsUpdate(UnreadNotificationsUpdate notificationsUpdate, String... expectedNotifications) { assertThat(notificationsUpdate.getNotifications()).extracting(Notification::getText).containsOnly(expectedNotifications); assertThat(notificationsUpdate.getNotifications()).extracting(Notification::getType).containsOnly(DEFAULT_NOTIFICATION_TYPE); diff --git a/application/src/test/java/org/thingsboard/server/service/notification/MockNotificationSettingsService.java b/application/src/test/java/org/thingsboard/server/service/notification/TestNotificationSettingsService.java similarity index 61% rename from application/src/test/java/org/thingsboard/server/service/notification/MockNotificationSettingsService.java rename to application/src/test/java/org/thingsboard/server/service/notification/TestNotificationSettingsService.java index 6c3a02051e..3b21b35a73 100644 --- a/application/src/test/java/org/thingsboard/server/service/notification/MockNotificationSettingsService.java +++ b/application/src/test/java/org/thingsboard/server/service/notification/TestNotificationSettingsService.java @@ -19,14 +19,20 @@ import org.springframework.context.annotation.Primary; import org.springframework.stereotype.Service; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.dao.notification.DefaultNotificationSettingsService; +import org.thingsboard.server.dao.notification.NotificationTargetService; +import org.thingsboard.server.dao.notification.NotificationTemplateService; import org.thingsboard.server.dao.settings.AdminSettingsService; +import org.thingsboard.server.dao.user.UserService; @Service @Primary -public class MockNotificationSettingsService extends DefaultNotificationSettingsService { +public class TestNotificationSettingsService extends DefaultNotificationSettingsService { - public MockNotificationSettingsService(AdminSettingsService adminSettingsService) { - super(adminSettingsService, null, null, null, null, null); + public TestNotificationSettingsService(AdminSettingsService adminSettingsService, + NotificationTargetService notificationTargetService, + NotificationTemplateService notificationTemplateService, + UserService userService) { + super(adminSettingsService, notificationTargetService, notificationTemplateService, null, userService); } @Override diff --git a/application/src/test/resources/application-test.properties b/application/src/test/resources/application-test.properties index ad86ff736b..28c13e33fb 100644 --- a/application/src/test/resources/application-test.properties +++ b/application/src/test/resources/application-test.properties @@ -67,3 +67,5 @@ sql.ttl.audit_logs.ttl=2592000 sql.edge_events.partition_size=168 sql.ttl.edge_events.edge_event_ttl=2592000 + +server.log_controller_error_stack_trace=false 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 619e1ad38f..691ecf8bc1 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 @@ -54,7 +54,6 @@ public class NotificationRequestStats { public void reportSent(NotificationDeliveryMethod deliveryMethod, NotificationRecipient recipient) { sent.computeIfAbsent(deliveryMethod, k -> new AtomicInteger()).incrementAndGet(); - processedRecipients.computeIfAbsent(deliveryMethod, k -> ConcurrentHashMap.newKeySet()).add(recipient.getId()); } public void reportError(NotificationDeliveryMethod deliveryMethod, Throwable error, NotificationRecipient recipient) { @@ -68,6 +67,10 @@ public class NotificationRequestStats { errors.computeIfAbsent(deliveryMethod, k -> new ConcurrentHashMap<>()).put(recipient.getTitle(), errorMessage); } + public void reportProcessed(NotificationDeliveryMethod deliveryMethod, Object recipientId) { + processedRecipients.computeIfAbsent(deliveryMethod, k -> ConcurrentHashMap.newKeySet()).add(recipientId); + } + public boolean contains(NotificationDeliveryMethod deliveryMethod, Object recipientId) { Set processedRecipients = this.processedRecipients.get(deliveryMethod); return processedRecipients != null && processedRecipients.contains(recipientId); diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/notification/settings/UserNotificationSettings.java b/common/data/src/main/java/org/thingsboard/server/common/data/notification/settings/UserNotificationSettings.java index 9e2f2740ab..f24520bc32 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/notification/settings/UserNotificationSettings.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/notification/settings/UserNotificationSettings.java @@ -16,57 +16,61 @@ package org.thingsboard.server.common.data.notification.settings; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Data; -import org.thingsboard.server.common.data.id.NotificationRuleId; import org.thingsboard.server.common.data.notification.NotificationDeliveryMethod; -import org.thingsboard.server.common.data.notification.rule.NotificationRule; +import org.thingsboard.server.common.data.notification.NotificationType; +import org.thingsboard.server.common.data.notification.targets.NotificationTargetType; import javax.validation.Valid; +import javax.validation.constraints.AssertTrue; import javax.validation.constraints.NotNull; import java.util.Collections; -import java.util.List; +import java.util.Map; import java.util.Set; -import java.util.UUID; @Data public class UserNotificationSettings { @NotNull @Valid - private final List prefs; + private final Map prefs; - public static final UserNotificationSettings DEFAULT = new UserNotificationSettings(Collections.emptyList()); + public static final UserNotificationSettings DEFAULT = new UserNotificationSettings(Collections.emptyMap()); @JsonCreator - public UserNotificationSettings(@JsonProperty("prefs") List prefs) { + public UserNotificationSettings(@JsonProperty("prefs") Map prefs) { this.prefs = prefs; } - public Set getEnabledDeliveryMethods(NotificationRuleId ruleId) { - return prefs.stream() - .filter(pref -> pref.getRuleId().equals(ruleId.getId())).findFirst() - .map(pref -> pref.isEnabled() ? pref.getEnabledDeliveryMethods() : Collections.emptySet()) - .orElse(NotificationDeliveryMethod.values); + public Set getEnabledDeliveryMethods(NotificationType notificationType) { + NotificationPref pref = prefs.get(notificationType); + if (pref != null) { + return pref.isEnabled() ? pref.getEnabledDeliveryMethods() : Collections.emptySet(); + } else { + return NotificationDeliveryMethod.values; + } } @Data public static class NotificationPref { - @NotNull - private UUID ruleId; - private String ruleName; private boolean enabled; @NotNull private Set enabledDeliveryMethods; - public static NotificationPref createDefault(NotificationRule rule) { + public static NotificationPref createDefault() { NotificationPref pref = new NotificationPref(); - pref.setRuleId(rule.getUuidId()); - pref.setRuleName(rule.getName()); pref.setEnabled(true); pref.setEnabledDeliveryMethods(NotificationDeliveryMethod.values); return pref; } + + @JsonIgnore + @AssertTrue(message = "Only email, Web and SMS delivery methods are allowed") + public boolean isValid() { + return NotificationTargetType.PLATFORM_USERS.getSupportedDeliveryMethods().containsAll(enabledDeliveryMethods); + } } } 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 af9687840a..8fd15e6cd0 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 @@ -20,7 +20,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import lombok.RequiredArgsConstructor; import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; -import org.springframework.data.domain.Sort; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; @@ -31,7 +30,6 @@ import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.notification.NotificationType; -import org.thingsboard.server.common.data.notification.rule.NotificationRule; import org.thingsboard.server.common.data.notification.settings.NotificationSettings; import org.thingsboard.server.common.data.notification.settings.UserNotificationSettings; import org.thingsboard.server.common.data.notification.settings.UserNotificationSettings.NotificationPref; @@ -46,17 +44,14 @@ import org.thingsboard.server.common.data.notification.targets.platform.TenantAd import org.thingsboard.server.common.data.notification.targets.platform.UsersFilter; import org.thingsboard.server.common.data.notification.targets.platform.UsersFilterType; import org.thingsboard.server.common.data.page.PageLink; -import org.thingsboard.server.common.data.page.SortOrder; import org.thingsboard.server.dao.settings.AdminSettingsService; import org.thingsboard.server.dao.user.UserService; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; +import java.util.EnumMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.UUID; import static java.util.function.Predicate.not; @@ -67,11 +62,11 @@ public class DefaultNotificationSettingsService implements NotificationSettingsS private final AdminSettingsService adminSettingsService; private final NotificationTargetService notificationTargetService; private final NotificationTemplateService notificationTemplateService; - private final NotificationRuleService notificationRuleService; private final DefaultNotifications defaultNotifications; private final UserService userService; private static final String SETTINGS_KEY = "notifications"; + private static final String USER_SETTINGS_KEY = "notificationSettings"; @CacheEvict(cacheNames = CacheConstants.NOTIFICATION_SETTINGS_CACHE, key = "#tenantId") @Override @@ -103,47 +98,30 @@ public class DefaultNotificationSettingsService implements NotificationSettingsS public void saveUserNotificationSettings(TenantId tenantId, UserId userId, UserNotificationSettings settings) { User user = userService.findUserById(tenantId, userId); ObjectNode additionalInfo = (ObjectNode) Optional.ofNullable(user.getAdditionalInfo()).orElseGet(JacksonUtil::newObjectNode); - additionalInfo.set("notificationSettings", JacksonUtil.valueToTree(settings)); + additionalInfo.set(USER_SETTINGS_KEY, JacksonUtil.valueToTree(settings)); user.setAdditionalInfo(additionalInfo); userService.saveUser(user); } @Override public UserNotificationSettings getUserNotificationSettings(TenantId tenantId, User user, boolean format) { - UserNotificationSettings settings = Optional.ofNullable(user.getAdditionalInfo().get("notificationSettings")) + UserNotificationSettings settings = Optional.ofNullable(user.getAdditionalInfo().get(USER_SETTINGS_KEY)) .filter(not(JsonNode::isNull)) .map(json -> JacksonUtil.treeToValue(json, UserNotificationSettings.class)) .orElse(null); if (!format) { - if (settings != null) { - return settings; - } else { - return UserNotificationSettings.DEFAULT; - } + return Optional.ofNullable(settings).orElse(UserNotificationSettings.DEFAULT); } - Map rules = new HashMap<>(); - notificationRuleService.findNotificationRulesByTenantId(tenantId, new PageLink(Integer.MAX_VALUE, 0,null, SortOrder.byCreatedTimeDesc)) - .getData().forEach(rule -> rules.put(rule.getUuidId(), rule)); - - List prefs = new ArrayList<>(); - if (settings == null) { - rules.values().forEach(rule -> { - prefs.add(NotificationPref.createDefault(rule)); - }); - } else { - settings.getPrefs().forEach(pref -> { - NotificationRule rule = rules.remove(pref.getRuleId()); - if (rule == null) { - return; - } - pref.setRuleName(rule.getName()); - prefs.add(pref); - }); - rules.values().forEach(rule -> { - prefs.add(NotificationPref.createDefault(rule)); - }); + Map prefs = new EnumMap<>(NotificationType.class); + if (settings != null) { + prefs.putAll(settings.getPrefs()); } + NotificationPref defaultPref = NotificationPref.createDefault(); + for (NotificationType notificationType : NotificationType.values()) { + prefs.putIfAbsent(notificationType, defaultPref); + } + prefs.remove(NotificationType.GENERAL); return new UserNotificationSettings(prefs); }