From ff18b8712a357f9af1545532a237e8ed00e9d6fe Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Wed, 6 Mar 2024 14:03:05 +0200 Subject: [PATCH] Fix null values in Firebase message data --- .../DefaultNotificationCenter.java | 12 ++++- .../MobileAppNotificationChannel.java | 5 +- .../AbstractNotificationApiTest.java | 13 +++-- .../notification/NotificationApiTest.java | 54 +++++++++++++++++++ .../notification/NotificationRuleApiTest.java | 4 +- .../NotificationRequestStats.java | 10 ++++ 6 files changed, 89 insertions(+), 9 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/notification/DefaultNotificationCenter.java b/application/src/main/java/org/thingsboard/server/service/notification/DefaultNotificationCenter.java index 5a9e3fc3e1..25a344b775 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 @@ -21,6 +21,7 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.thingsboard.rule.engine.api.NotificationCenter; +import org.thingsboard.server.cache.limits.RateLimitService; import org.thingsboard.server.common.data.EntityType; import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.id.EntityId; @@ -62,7 +63,6 @@ import org.thingsboard.server.dao.notification.NotificationService; import org.thingsboard.server.dao.notification.NotificationSettingsService; import org.thingsboard.server.dao.notification.NotificationTargetService; import org.thingsboard.server.dao.notification.NotificationTemplateService; -import org.thingsboard.server.cache.limits.RateLimitService; import org.thingsboard.server.gen.transport.TransportProtos; import org.thingsboard.server.queue.common.TbProtoQueueMsg; import org.thingsboard.server.queue.discovery.TopicService; @@ -204,6 +204,7 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple private void processNotificationRequestAsync(NotificationProcessingContext ctx, List targets, FutureCallback callback) { notificationExecutor.submit(() -> { + long startTs = System.currentTimeMillis(); NotificationRequestId requestId = ctx.getRequest().getId(); for (NotificationTarget target : targets) { try { @@ -219,9 +220,16 @@ public class DefaultNotificationCenter extends AbstractSubscriptionService imple return; } } - log.debug("[{}] Notification request processing is finished", requestId); NotificationRequestStats stats = ctx.getStats(); + long time = System.currentTimeMillis() - startTs; + int sent = stats.getTotalSent().get(); + int errors = stats.getTotalErrors().get(); + if (errors > 0) { + log.info("[{}][{}] Notification request processing finished in {} ms (sent: {}, errors: {})", ctx.getTenantId(), requestId, time, sent, errors); + } else { + log.info("[{}][{}] Notification request processing finished in {} ms (sent: {})", ctx.getTenantId(), requestId, time, sent); + } updateRequestStats(ctx, requestId, stats); if (callback != null) { callback.onSuccess(stats); diff --git a/application/src/main/java/org/thingsboard/server/service/notification/channels/MobileAppNotificationChannel.java b/application/src/main/java/org/thingsboard/server/service/notification/channels/MobileAppNotificationChannel.java index dc7f3f9d99..646556372a 100644 --- a/application/src/main/java/org/thingsboard/server/service/notification/channels/MobileAppNotificationChannel.java +++ b/application/src/main/java/org/thingsboard/server/service/notification/channels/MobileAppNotificationChannel.java @@ -15,6 +15,8 @@ */ package org.thingsboard.server.service.notification.channels; +import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.base.Strings; import com.google.firebase.messaging.FirebaseMessagingException; import com.google.firebase.messaging.MessagingErrorCode; import lombok.RequiredArgsConstructor; @@ -82,7 +84,7 @@ public class MobileAppNotificationChannel implements NotificationChannel getNotificationData(MobileAppDeliveryMethodNotificationTemplate processedTemplate, NotificationProcessingContext ctx) { Map data = Optional.ofNullable(processedTemplate.getAdditionalConfig()) - .map(JacksonUtil::toFlatMap).orElseGet(HashMap::new); + .filter(JsonNode::isObject).map(JacksonUtil::toFlatMap).orElseGet(HashMap::new); NotificationInfo info = ctx.getRequest().getInfo(); if (info == null) { return data; @@ -107,6 +109,7 @@ public class MobileAppNotificationChannel implements NotificationChannel Strings.nullToEmpty(value)); return data; } diff --git a/application/src/test/java/org/thingsboard/server/service/notification/AbstractNotificationApiTest.java b/application/src/test/java/org/thingsboard/server/service/notification/AbstractNotificationApiTest.java index f54b5d9b7b..b945c162df 100644 --- a/application/src/test/java/org/thingsboard/server/service/notification/AbstractNotificationApiTest.java +++ b/application/src/test/java/org/thingsboard/server/service/notification/AbstractNotificationApiTest.java @@ -60,6 +60,7 @@ import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.Authority; import org.thingsboard.server.controller.AbstractControllerTest; import org.thingsboard.server.dao.DaoUtil; +import org.thingsboard.server.dao.notification.DefaultNotifications; import org.thingsboard.server.dao.notification.NotificationRequestService; import org.thingsboard.server.dao.notification.NotificationRuleService; import org.thingsboard.server.dao.notification.NotificationSettingsService; @@ -99,6 +100,8 @@ public abstract class AbstractNotificationApiTest extends AbstractControllerTest protected NotificationSettingsService notificationSettingsService; @Autowired protected SqlPartitioningRepository partitioningRepository; + @Autowired + protected DefaultNotifications defaultNotifications; public static final String DEFAULT_NOTIFICATION_SUBJECT = "Just a test"; public static final NotificationType DEFAULT_NOTIFICATION_TYPE = NotificationType.GENERAL; @@ -249,10 +252,14 @@ public abstract class AbstractNotificationApiTest extends AbstractControllerTest } protected NotificationRule createNotificationRule(NotificationRuleTriggerConfig triggerConfig, String subject, String text, NotificationTargetId... targets) { - NotificationTemplate template = createNotificationTemplate(NotificationType.valueOf(triggerConfig.getTriggerType().toString()), subject, text, NotificationDeliveryMethod.WEB); + return createNotificationRule(triggerConfig, subject, text, List.of(targets), NotificationDeliveryMethod.WEB); + } + + protected NotificationRule createNotificationRule(NotificationRuleTriggerConfig triggerConfig, String subject, String text, List targets, NotificationDeliveryMethod... deliveryMethods) { + NotificationTemplate template = createNotificationTemplate(NotificationType.valueOf(triggerConfig.getTriggerType().toString()), subject, text, deliveryMethods); NotificationRule rule = new NotificationRule(); - rule.setName(triggerConfig.getTriggerType() + " " + Arrays.toString(targets)); + rule.setName(triggerConfig.getTriggerType() + " " + targets); rule.setEnabled(true); rule.setTemplateId(template.getId()); rule.setTriggerType(triggerConfig.getTriggerType()); @@ -260,7 +267,7 @@ public abstract class AbstractNotificationApiTest extends AbstractControllerTest DefaultNotificationRuleRecipientsConfig recipientsConfig = new DefaultNotificationRuleRecipientsConfig(); recipientsConfig.setTriggerType(triggerConfig.getTriggerType()); - recipientsConfig.setTargets(DaoUtil.toUUIDs(List.of(targets))); + recipientsConfig.setTargets(DaoUtil.toUUIDs(targets)); rule.setRecipientsConfig(recipientsConfig); return saveNotificationRule(rule); 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 2d93573a25..d3b79029ba 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 @@ -30,8 +30,12 @@ import org.springframework.web.client.RestTemplate; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.rule.engine.api.NotificationCenter; import org.thingsboard.rule.engine.api.notification.FirebaseService; +import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.EntityType; import org.thingsboard.server.common.data.User; +import org.thingsboard.server.common.data.alarm.Alarm; +import org.thingsboard.server.common.data.alarm.AlarmComment; +import org.thingsboard.server.common.data.alarm.AlarmSeverity; import org.thingsboard.server.common.data.audit.ActionType; import org.thingsboard.server.common.data.id.DeviceId; import org.thingsboard.server.common.data.id.NotificationRequestId; @@ -49,6 +53,7 @@ import org.thingsboard.server.common.data.notification.NotificationRequestStats; import org.thingsboard.server.common.data.notification.NotificationRequestStatus; import org.thingsboard.server.common.data.notification.NotificationType; import org.thingsboard.server.common.data.notification.info.EntityActionNotificationInfo; +import org.thingsboard.server.common.data.notification.rule.trigger.config.AlarmCommentNotificationRuleTriggerConfig; import org.thingsboard.server.common.data.notification.settings.MobileAppNotificationDeliveryMethodConfig; import org.thingsboard.server.common.data.notification.settings.NotificationSettings; import org.thingsboard.server.common.data.notification.settings.SlackNotificationDeliveryMethodConfig; @@ -771,6 +776,55 @@ public class NotificationApiTest extends AbstractNotificationApiTest { verifyNoMoreInteractions(firebaseService); } + @Test + public void testMobileAppNotifications_ruleBased() throws Exception { + loginSysAdmin(); + MobileAppNotificationDeliveryMethodConfig config = new MobileAppNotificationDeliveryMethodConfig(); + config.setFirebaseServiceAccountCredentials("testCredentials"); + saveNotificationSettings(config); + + loginTenantAdmin(); + mobileToken = "tenantFcmToken"; + doPost("/api/user/mobile/session", new MobileSessionInfo()).andExpect(status().isOk()); + + createNotificationRule(AlarmCommentNotificationRuleTriggerConfig.builder().onlyUserComments(true).build(), + DefaultNotifications.alarmComment.getSubject(), DefaultNotifications.alarmComment.getText(), + List.of(createNotificationTarget(tenantAdminUserId).getId()), NotificationDeliveryMethod.MOBILE_APP); + + Device device = createDevice("test", "test"); + UUID alarmDashboardId = UUID.randomUUID(); + Alarm alarm = Alarm.builder() + .type("test") + .tenantId(tenantId) + .originator(device.getId()) + .severity(AlarmSeverity.MAJOR) + .details(JacksonUtil.newObjectNode() + .put("dashboardId", alarmDashboardId.toString())) + .build(); + alarm = doPost("/api/alarm", alarm, Alarm.class); + + AlarmComment comment = new AlarmComment(); + comment.setComment(JacksonUtil.newObjectNode() + .put("text", "text")); + doPost("/api/alarm/" + alarm.getId() + "/comment", comment, AlarmComment.class); + + ArgumentCaptor> msgCaptor = ArgumentCaptor.forClass(Map.class); + await().atMost(30, TimeUnit.SECONDS).untilAsserted(() -> { + verify(firebaseService).sendMessage(eq(tenantId), eq("testCredentials"), + eq("tenantFcmToken"), eq("Comment on 'test' alarm"), + eq(TENANT_ADMIN_EMAIL + " added comment: text"), + msgCaptor.capture()); + }); + Map firebaseMessageData = msgCaptor.getValue(); + assertThat(firebaseMessageData.keySet()).doesNotContainNull().doesNotContain(""); + assertThat(firebaseMessageData.values()).doesNotContainNull(); + assertThat(firebaseMessageData.get("info.userEmail")).isEqualTo(TENANT_ADMIN_EMAIL); + assertThat(firebaseMessageData.get("info.alarmType")).isEqualTo("test"); + assertThat(firebaseMessageData.get("onClick.enabled")).isEqualTo("true"); + assertThat(firebaseMessageData.get("onClick.linkType")).isEqualTo("DASHBOARD"); + assertThat(firebaseMessageData.get("onClick.dashboardId")).isEqualTo(alarmDashboardId.toString()); + } + @Test public void testMobileSettings_tenantLevel() throws Exception { MobileAppNotificationDeliveryMethodConfig config = new MobileAppNotificationDeliveryMethodConfig(); 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 1e95f9add3..9cf73894c2 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 @@ -26,6 +26,7 @@ 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.server.cache.limits.RateLimitService; import org.thingsboard.server.common.data.DataConstants; import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.DeviceProfile; @@ -93,7 +94,6 @@ import org.thingsboard.server.dao.notification.DefaultNotifications; import org.thingsboard.server.dao.notification.NotificationRequestService; import org.thingsboard.server.dao.rule.RuleChainService; import org.thingsboard.server.dao.service.DaoSqlTest; -import org.thingsboard.server.cache.limits.RateLimitService; import org.thingsboard.server.queue.notification.DefaultNotificationDeduplicationService; import org.thingsboard.server.service.notification.rule.cache.DefaultNotificationRulesCache; import org.thingsboard.server.service.state.DeviceStateService; @@ -140,8 +140,6 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest { @Autowired private NotificationRuleProcessor notificationRuleProcessor; @Autowired - private DefaultNotifications defaultNotifications; - @Autowired private DefaultNotificationRulesCache notificationRulesCache; @Autowired private DeviceStateService deviceStateService; 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 f7cdbda6fc..2bd9c526fd 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 @@ -31,14 +31,20 @@ import java.util.concurrent.atomic.AtomicInteger; public class NotificationRequestStats { private final Map sent; + @JsonIgnore + private final AtomicInteger totalSent; private final Map> errors; + @JsonIgnore + private final AtomicInteger totalErrors; private String error; @JsonIgnore private final Map> processedRecipients; public NotificationRequestStats() { this.sent = new ConcurrentHashMap<>(); + this.totalSent = new AtomicInteger(); this.errors = new ConcurrentHashMap<>(); + this.totalErrors = new AtomicInteger(); this.processedRecipients = new ConcurrentHashMap<>(); } @@ -47,13 +53,16 @@ public class NotificationRequestStats { @JsonProperty("errors") Map> errors, @JsonProperty("error") String error) { this.sent = sent; + this.totalSent = null; this.errors = errors; + this.totalErrors = null; this.error = error; this.processedRecipients = Collections.emptyMap(); } public void reportSent(NotificationDeliveryMethod deliveryMethod, NotificationRecipient recipient) { sent.computeIfAbsent(deliveryMethod, k -> new AtomicInteger()).incrementAndGet(); + totalSent.incrementAndGet(); } public void reportError(NotificationDeliveryMethod deliveryMethod, Throwable error, NotificationRecipient recipient) { @@ -65,6 +74,7 @@ public class NotificationRequestStats { errorMessage = error.getClass().getSimpleName(); } errors.computeIfAbsent(deliveryMethod, k -> new ConcurrentHashMap<>()).put(recipient.getTitle(), errorMessage); + totalErrors.incrementAndGet(); } public void reportProcessed(NotificationDeliveryMethod deliveryMethod, Object recipientId) {