From 6a3beb9213329bc2d305207ea555c26dce5c1292 Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Fri, 29 Sep 2023 16:00:39 +0300 Subject: [PATCH 1/2] Remove foreign keys for notification table --- .../main/data/upgrade/3.6.0/schema_update.sql | 26 +++++++++ .../notification/NotificationApiTest.java | 53 +++++++++++++++++++ .../notification/NotificationRuleApiTest.java | 4 +- .../DefaultNotificationRequestService.java | 3 +- .../dao/notification/NotificationDao.java | 4 ++ .../sql/notification/JpaNotificationDao.java | 10 ++++ .../notification/NotificationRepository.java | 6 +++ .../server/dao/user/UserServiceImpl.java | 3 ++ .../resources/sql/schema-entities-idx.sql | 4 ++ .../main/resources/sql/schema-entities.sql | 4 +- 10 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 application/src/main/data/upgrade/3.6.0/schema_update.sql diff --git a/application/src/main/data/upgrade/3.6.0/schema_update.sql b/application/src/main/data/upgrade/3.6.0/schema_update.sql new file mode 100644 index 0000000000..0ad2d97b7a --- /dev/null +++ b/application/src/main/data/upgrade/3.6.0/schema_update.sql @@ -0,0 +1,26 @@ +-- +-- 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. +-- + +ALTER TABLE widget_type + ADD COLUMN IF NOT EXISTS tags text[]; + +ALTER TABLE api_usage_state ADD COLUMN IF NOT EXISTS tbel_exec varchar(32); +UPDATE api_usage_state SET tbel_exec = js_exec WHERE tbel_exec IS NULL; + +ALTER TABLE notification DROP CONSTRAINT IF EXISTS fk_notification_request_id; +ALTER TABLE notification DROP CONSTRAINT IF EXISTS fk_notification_recipient_id; +CREATE INDEX IF NOT EXISTS idx_notification_notification_request_id ON notification(request_id); +CREATE INDEX IF NOT EXISTS idx_notification_request_tenant_id ON notification_request(tenant_id); 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 1b3fce3c3c..54b3079465 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 @@ -25,9 +25,11 @@ import org.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.web.client.RestTemplate; import org.thingsboard.rule.engine.api.NotificationCenter; +import org.thingsboard.server.common.data.EntityType; import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.audit.ActionType; import org.thingsboard.server.common.data.id.DeviceId; +import org.thingsboard.server.common.data.id.NotificationRequestId; import org.thingsboard.server.common.data.id.NotificationRuleId; import org.thingsboard.server.common.data.id.NotificationTargetId; import org.thingsboard.server.common.data.id.TenantId; @@ -62,6 +64,7 @@ import org.thingsboard.server.common.data.notification.template.NotificationTemp import org.thingsboard.server.common.data.notification.template.SlackDeliveryMethodNotificationTemplate; import org.thingsboard.server.common.data.notification.template.SmsDeliveryMethodNotificationTemplate; import org.thingsboard.server.common.data.notification.template.WebDeliveryMethodNotificationTemplate; +import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.Authority; import org.thingsboard.server.dao.notification.DefaultNotifications; import org.thingsboard.server.dao.notification.NotificationDao; @@ -74,6 +77,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.UUID; import java.util.concurrent.TimeUnit; @@ -86,6 +90,7 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @DaoSqlTest @Slf4j @@ -279,6 +284,49 @@ public class NotificationApiTest extends AbstractNotificationApiTest { assertThat(getMyNotifications(false, 10)).size().isZero(); } + @Test + public void whenTenantIsDeleted_thenDeleteNotifications() throws Exception { + createDifferentTenant(); + NotificationTarget target = createNotificationTarget(savedDifferentTenantUser.getId()); + int notificationsCount = 20; + for (int i = 0; i < notificationsCount; i++) { + NotificationRequest request = submitNotificationRequest(target.getId(), "Test " + i, NotificationDeliveryMethod.WEB); + awaitNotificationRequest(request.getId()); + } + List requests = notificationRequestService.findNotificationRequestsByTenantIdAndOriginatorType(differentTenantId, EntityType.USER, new PageLink(100)).getData(); + assertThat(requests).size().isEqualTo(notificationsCount); + for (NotificationRequest request : requests) { + List notifications = notificationDao.findByRequestId(differentTenantId, request.getId(), new PageLink(100)).getData(); + assertThat(notifications).size().isNotZero(); + } + + deleteDifferentTenant(); + + assertThat(notificationRequestService.findNotificationRequestsByTenantIdAndOriginatorType(differentTenantId, EntityType.USER, new PageLink(1)).getTotalElements()) + .isZero(); + for (NotificationRequest request : requests) { + assertThat(notificationDao.findByRequestId(differentTenantId, request.getId(), new PageLink(100)).getTotalElements()) + .isZero(); + } + } + + @Test + public void whenUserIsDeleted_thenDeleteNotifications() throws Exception { + NotificationTarget target = createNotificationTarget(customerUserId); + int notificationsCount = 20; + for (int i = 0; i < notificationsCount; i++) { + NotificationRequest request = submitNotificationRequest(target.getId(), "Test " + i, NotificationDeliveryMethod.WEB); + awaitNotificationRequest(request.getId()); + } + List notifications = notificationDao.findByRecipientIdAndPageLink(tenantId, customerUserId, new PageLink(100)).getData(); + assertThat(notifications).size().isGreaterThanOrEqualTo(notificationsCount); + + doDelete("/api/user/" + customerUserId).andExpect(status().isOk()); + + notifications = notificationDao.findByRecipientIdAndPageLink(tenantId, customerUserId, new PageLink(100)).getData(); + assertThat(notifications).isEmpty(); + } + @Test public void testNotificationUpdatesForSeveralUsers() throws Exception { int usersCount = 150; @@ -692,6 +740,11 @@ public class NotificationApiTest extends AbstractNotificationApiTest { return future.get(30, TimeUnit.SECONDS); } + private NotificationRequestStats awaitNotificationRequest(NotificationRequestId requestId) { + return await().atMost(30, TimeUnit.SECONDS) + .until(() -> getStats(requestId), Objects::nonNull); + } + 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/NotificationRuleApiTest.java b/application/src/test/java/org/thingsboard/server/service/notification/NotificationRuleApiTest.java index 0a2854a28d..d08b1a121b 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 @@ -456,7 +456,9 @@ public class NotificationRuleApiTest extends AbstractNotificationApiTest { loginSysAdmin(); notifications = await().atMost(30, TimeUnit.SECONDS) - .until(() -> getMyNotifications(true, 10), list -> list.size() == 1); + .until(() -> getMyNotifications(true, 10).stream() + .filter(notification -> notification.getType() == NotificationType.RATE_LIMITS) + .collect(Collectors.toList()), list -> list.size() == 1); assertThat(notifications).allSatisfy(notification -> { assertThat(notification.getSubject()).isEqualTo("Rate limits exceeded for tenant " + TEST_TENANT_NAME); }); diff --git a/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java b/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java index e951a75c7c..e388c189a0 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java @@ -42,6 +42,7 @@ import java.util.Optional; public class DefaultNotificationRequestService implements NotificationRequestService, EntityDaoService { private final NotificationRequestDao notificationRequestDao; + private final NotificationDao notificationDao; private final NotificationRequestValidator notificationRequestValidator = new NotificationRequestValidator(); @@ -81,10 +82,10 @@ public class DefaultNotificationRequestService implements NotificationRequestSer return notificationRequestDao.findByRuleIdAndOriginatorEntityId(tenantId, ruleId, originatorEntityId); } - // ON DELETE CASCADE is used: notifications for request are deleted as well @Override public void deleteNotificationRequest(TenantId tenantId, NotificationRequestId requestId) { notificationRequestDao.removeById(tenantId, requestId.getId()); + notificationDao.deleteByRequestId(tenantId, requestId); } @Override diff --git a/dao/src/main/java/org/thingsboard/server/dao/notification/NotificationDao.java b/dao/src/main/java/org/thingsboard/server/dao/notification/NotificationDao.java index 1ced714bce..fb3890fcbb 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/notification/NotificationDao.java +++ b/dao/src/main/java/org/thingsboard/server/dao/notification/NotificationDao.java @@ -41,4 +41,8 @@ public interface NotificationDao extends Dao { int updateStatusByRecipientId(TenantId tenantId, UserId recipientId, NotificationStatus status); + void deleteByRequestId(TenantId tenantId, NotificationRequestId requestId); + + void deleteByRecipientId(TenantId tenantId, UserId recipientId); + } diff --git a/dao/src/main/java/org/thingsboard/server/dao/sql/notification/JpaNotificationDao.java b/dao/src/main/java/org/thingsboard/server/dao/sql/notification/JpaNotificationDao.java index 7eeab8c592..21a5ab5c0c 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sql/notification/JpaNotificationDao.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sql/notification/JpaNotificationDao.java @@ -104,6 +104,16 @@ public class JpaNotificationDao extends JpaAbstractDao getEntityClass() { return NotificationEntity.class; diff --git a/dao/src/main/java/org/thingsboard/server/dao/sql/notification/NotificationRepository.java b/dao/src/main/java/org/thingsboard/server/dao/sql/notification/NotificationRepository.java index c08a3c290e..586af7d674 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sql/notification/NotificationRepository.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sql/notification/NotificationRepository.java @@ -61,6 +61,12 @@ public interface NotificationRepository extends JpaRepository userValidator; private final DataValidator userCredentialsValidator; private final ApplicationEventPublisher eventPublisher; @@ -255,6 +257,7 @@ public class UserServiceImpl extends AbstractEntityService implements UserServic UserCredentials userCredentials = userCredentialsDao.findByUserId(tenantId, userId.getId()); userCredentialsDao.removeById(tenantId, userCredentials.getUuidId()); userAuthSettingsDao.removeByUserId(userId); + notificationDao.deleteByRecipientId(tenantId, userId); deleteEntityRelations(tenantId, userId); userDao.removeById(tenantId, userId.getId()); eventPublisher.publishEvent(new UserCredentialsInvalidationEvent(userId)); diff --git a/dao/src/main/resources/sql/schema-entities-idx.sql b/dao/src/main/resources/sql/schema-entities-idx.sql index 675fcd3ec0..68eede5ce4 100644 --- a/dao/src/main/resources/sql/schema-entities-idx.sql +++ b/dao/src/main/resources/sql/schema-entities-idx.sql @@ -104,6 +104,8 @@ CREATE INDEX IF NOT EXISTS idx_notification_rule_tenant_id_trigger_type_created_ CREATE INDEX IF NOT EXISTS idx_notification_request_tenant_id_user_created_time ON notification_request(tenant_id, created_time DESC) WHERE originator_entity_type = 'USER'; +CREATE INDEX IF NOT EXISTS idx_notification_request_tenant_id ON notification_request(tenant_id); + CREATE INDEX IF NOT EXISTS idx_notification_request_rule_id_originator_entity_id ON notification_request(rule_id, originator_entity_id) WHERE originator_entity_type = 'ALARM'; @@ -112,6 +114,8 @@ CREATE INDEX IF NOT EXISTS idx_notification_request_status ON notification_reque CREATE INDEX IF NOT EXISTS idx_notification_id ON notification(id); +CREATE INDEX IF NOT EXISTS idx_notification_notification_request_id ON notification(request_id); + CREATE INDEX IF NOT EXISTS idx_notification_recipient_id_created_time ON notification(recipient_id, created_time DESC); CREATE INDEX IF NOT EXISTS idx_notification_recipient_id_unread ON notification(recipient_id) WHERE status <> 'READ'; diff --git a/dao/src/main/resources/sql/schema-entities.sql b/dao/src/main/resources/sql/schema-entities.sql index 4df73de9d6..bc49142e44 100644 --- a/dao/src/main/resources/sql/schema-entities.sql +++ b/dao/src/main/resources/sql/schema-entities.sql @@ -851,8 +851,8 @@ CREATE TABLE IF NOT EXISTS notification_request ( CREATE TABLE IF NOT EXISTS notification ( id UUID NOT NULL, created_time BIGINT NOT NULL, - request_id UUID NULL CONSTRAINT fk_notification_request_id REFERENCES notification_request(id) ON DELETE CASCADE, - recipient_id UUID NOT NULL CONSTRAINT fk_notification_recipient_id REFERENCES tb_user(id) ON DELETE CASCADE, + request_id UUID, + recipient_id UUID NOT NULL, type VARCHAR(50) NOT NULL, subject VARCHAR(255), body VARCHAR(1000) NOT NULL, From c0294e387854de892920ad4540f637912371e01f Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Tue, 3 Oct 2023 17:29:28 +0300 Subject: [PATCH 2/2] Don't delete notifications when recipient is deleted --- .../service/ttl/AlarmsCleanUpService.java | 1 - .../ttl/NotificationsCleanUpService.java | 4 +-- .../notification/NotificationApiTest.java | 28 +------------------ .../DefaultNotificationRequestService.java | 1 + .../insert/sql/SqlPartitioningRepository.java | 2 +- .../server/dao/user/UserServiceImpl.java | 3 -- 6 files changed, 5 insertions(+), 34 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/ttl/AlarmsCleanUpService.java b/application/src/main/java/org/thingsboard/server/service/ttl/AlarmsCleanUpService.java index 2f84767eff..416f95cf96 100644 --- a/application/src/main/java/org/thingsboard/server/service/ttl/AlarmsCleanUpService.java +++ b/application/src/main/java/org/thingsboard/server/service/ttl/AlarmsCleanUpService.java @@ -92,7 +92,6 @@ public class AlarmsCleanUpService { while (true) { PageData toRemove = alarmDao.findAlarmsIdsByEndTsBeforeAndTenantId(expirationTime, tenantId, removalBatchRequest); for (AlarmId alarmId : toRemove.getData()) { - relationService.deleteEntityRelations(tenantId, alarmId); Alarm alarm = alarmService.delAlarm(tenantId, alarmId, false).getAlarm(); if (alarm != null) { entityActionService.pushEntityActionToRuleEngine(alarm.getOriginator(), alarm, tenantId, null, ActionType.ALARM_DELETE, null); diff --git a/application/src/main/java/org/thingsboard/server/service/ttl/NotificationsCleanUpService.java b/application/src/main/java/org/thingsboard/server/service/ttl/NotificationsCleanUpService.java index 65b04ef0ab..b7e6e57e04 100644 --- a/application/src/main/java/org/thingsboard/server/service/ttl/NotificationsCleanUpService.java +++ b/application/src/main/java/org/thingsboard/server/service/ttl/NotificationsCleanUpService.java @@ -63,8 +63,8 @@ public class NotificationsCleanUpService extends AbstractCleanUpService { if (lastRemovedNotificationTs > 0) { long gap = TimeUnit.MINUTES.toMillis(10); long requestExpTime = lastRemovedNotificationTs - TimeUnit.SECONDS.toMillis(NotificationRequestConfig.MAX_SENDING_DELAY) - gap; - // TODO: double-check this - notificationRequestDao.removeAllByCreatedTimeBefore(requestExpTime); + int removed = notificationRequestDao.removeAllByCreatedTimeBefore(requestExpTime); + log.info("Removed {} outdated notification requests older than {}", removed, requestExpTime); } } 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 54b3079465..21d29833b9 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 @@ -90,7 +90,6 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @DaoSqlTest @Slf4j @@ -285,7 +284,7 @@ public class NotificationApiTest extends AbstractNotificationApiTest { } @Test - public void whenTenantIsDeleted_thenDeleteNotifications() throws Exception { + public void whenTenantIsDeleted_thenDeleteNotificationRequests() throws Exception { createDifferentTenant(); NotificationTarget target = createNotificationTarget(savedDifferentTenantUser.getId()); int notificationsCount = 20; @@ -295,36 +294,11 @@ public class NotificationApiTest extends AbstractNotificationApiTest { } List requests = notificationRequestService.findNotificationRequestsByTenantIdAndOriginatorType(differentTenantId, EntityType.USER, new PageLink(100)).getData(); assertThat(requests).size().isEqualTo(notificationsCount); - for (NotificationRequest request : requests) { - List notifications = notificationDao.findByRequestId(differentTenantId, request.getId(), new PageLink(100)).getData(); - assertThat(notifications).size().isNotZero(); - } deleteDifferentTenant(); assertThat(notificationRequestService.findNotificationRequestsByTenantIdAndOriginatorType(differentTenantId, EntityType.USER, new PageLink(1)).getTotalElements()) .isZero(); - for (NotificationRequest request : requests) { - assertThat(notificationDao.findByRequestId(differentTenantId, request.getId(), new PageLink(100)).getTotalElements()) - .isZero(); - } - } - - @Test - public void whenUserIsDeleted_thenDeleteNotifications() throws Exception { - NotificationTarget target = createNotificationTarget(customerUserId); - int notificationsCount = 20; - for (int i = 0; i < notificationsCount; i++) { - NotificationRequest request = submitNotificationRequest(target.getId(), "Test " + i, NotificationDeliveryMethod.WEB); - awaitNotificationRequest(request.getId()); - } - List notifications = notificationDao.findByRecipientIdAndPageLink(tenantId, customerUserId, new PageLink(100)).getData(); - assertThat(notifications).size().isGreaterThanOrEqualTo(notificationsCount); - - doDelete("/api/user/" + customerUserId).andExpect(status().isOk()); - - notifications = notificationDao.findByRecipientIdAndPageLink(tenantId, customerUserId, new PageLink(100)).getData(); - assertThat(notifications).isEmpty(); } @Test diff --git a/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java b/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java index e388c189a0..0a11c058e8 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/notification/DefaultNotificationRequestService.java @@ -98,6 +98,7 @@ public class DefaultNotificationRequestService implements NotificationRequestSer notificationRequestDao.updateById(tenantId, requestId, requestStatus, stats); } + // notifications themselves are left in the database until removed by ttl @Override public void deleteNotificationRequestsByTenantId(TenantId tenantId) { notificationRequestDao.removeByTenantId(tenantId); diff --git a/dao/src/main/java/org/thingsboard/server/dao/sqlts/insert/sql/SqlPartitioningRepository.java b/dao/src/main/java/org/thingsboard/server/dao/sqlts/insert/sql/SqlPartitioningRepository.java index 60614e8c90..e317e6d7c0 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sqlts/insert/sql/SqlPartitioningRepository.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sqlts/insert/sql/SqlPartitioningRepository.java @@ -140,7 +140,7 @@ public class SqlPartitioningRepository { try { partitions.add(Long.parseLong(partitionTsStr)); } catch (NumberFormatException nfe) { - log.warn("Failed to parse table name: {}", partitionTableName); + log.debug("Failed to parse table name: {}", partitionTableName); } } return partitions; diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java index 521b1dbd4d..9776e5fe51 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java @@ -49,7 +49,6 @@ import org.thingsboard.server.dao.eventsourcing.ActionEntityEvent; import org.thingsboard.server.dao.eventsourcing.DeleteEntityEvent; import org.thingsboard.server.dao.eventsourcing.SaveEntityEvent; import org.thingsboard.server.dao.exception.IncorrectParameterException; -import org.thingsboard.server.dao.notification.NotificationDao; import org.thingsboard.server.dao.service.DataValidator; import org.thingsboard.server.dao.service.PaginatedRemover; @@ -86,7 +85,6 @@ public class UserServiceImpl extends AbstractEntityService implements UserServic private final UserDao userDao; private final UserCredentialsDao userCredentialsDao; private final UserAuthSettingsDao userAuthSettingsDao; - private final NotificationDao notificationDao; private final DataValidator userValidator; private final DataValidator userCredentialsValidator; private final ApplicationEventPublisher eventPublisher; @@ -257,7 +255,6 @@ public class UserServiceImpl extends AbstractEntityService implements UserServic UserCredentials userCredentials = userCredentialsDao.findByUserId(tenantId, userId.getId()); userCredentialsDao.removeById(tenantId, userCredentials.getUuidId()); userAuthSettingsDao.removeByUserId(userId); - notificationDao.deleteByRecipientId(tenantId, userId); deleteEntityRelations(tenantId, userId); userDao.removeById(tenantId, userId.getId()); eventPublisher.publishEvent(new UserCredentialsInvalidationEvent(userId));