diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmController.java index a83e9e446e..dd8d883145 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmController.java @@ -157,7 +157,7 @@ public class AlarmController extends BaseController { @PreAuthorize("hasAnyAuthority('TENANT_ADMIN', 'CUSTOMER_USER')") @RequestMapping(value = "/alarm/{alarmId}", method = RequestMethod.DELETE) @ResponseBody - public Boolean deleteAlarm(@Parameter(description = ALARM_ID_PARAM_DESCRIPTION) @PathVariable(ALARM_ID) String strAlarmId) throws ThingsboardException { + public boolean deleteAlarm(@Parameter(description = ALARM_ID_PARAM_DESCRIPTION) @PathVariable(ALARM_ID) String strAlarmId) throws ThingsboardException { checkParameter(ALARM_ID, strAlarmId); AlarmId alarmId = new AlarmId(toUUID(strAlarmId)); Alarm alarm = checkAlarmId(alarmId, Operation.DELETE); diff --git a/application/src/main/java/org/thingsboard/server/service/entitiy/AbstractTbEntityService.java b/application/src/main/java/org/thingsboard/server/service/entitiy/AbstractTbEntityService.java index e9ee7202a7..476a4ef5ca 100644 --- a/application/src/main/java/org/thingsboard/server/service/entitiy/AbstractTbEntityService.java +++ b/application/src/main/java/org/thingsboard/server/service/entitiy/AbstractTbEntityService.java @@ -17,10 +17,8 @@ package org.thingsboard.server.service.entitiy; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Lazy; import org.springframework.core.env.Environment; import org.thingsboard.server.cluster.TbClusterService; @@ -31,16 +29,10 @@ import org.thingsboard.server.common.data.exception.ThingsboardException; import org.thingsboard.server.common.data.id.EntityId; import org.thingsboard.server.common.data.id.EntityIdFactory; import org.thingsboard.server.dao.alarm.AlarmService; -import org.thingsboard.server.dao.asset.AssetProfileService; -import org.thingsboard.server.dao.asset.AssetService; import org.thingsboard.server.dao.customer.CustomerService; -import org.thingsboard.server.dao.device.DeviceProfileService; -import org.thingsboard.server.dao.device.DeviceService; import org.thingsboard.server.dao.edge.EdgeService; import org.thingsboard.server.dao.entity.EntityService; import org.thingsboard.server.dao.model.ModelConstants; -import org.thingsboard.server.dao.tenant.TenantService; -import org.thingsboard.server.service.executors.DbCallbackExecutorService; import org.thingsboard.server.service.sync.vc.EntitiesVersionControlService; import org.thingsboard.server.service.telemetry.AlarmSubscriptionService; @@ -55,12 +47,6 @@ public abstract class AbstractTbEntityService { @Autowired private Environment env; - @Value("${server.log_controller_error_stack_trace}") - @Getter - private boolean logControllerErrorStackTrace; - - @Autowired - protected DbCallbackExecutorService dbExecutor; @Autowired(required = false) protected TbLogEntityActionService logEntityActionService; @Autowired(required = false) diff --git a/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmService.java b/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmService.java index 4766d8e039..5a1dee3bb5 100644 --- a/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmService.java +++ b/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmService.java @@ -190,11 +190,24 @@ public class DefaultTbAlarmService extends AbstractTbEntityService implements Tb } @Override - public Boolean delete(Alarm alarm, User user) { - TenantId tenantId = alarm.getTenantId(); - logEntityActionService.logEntityAction(tenantId, alarm.getOriginator(), alarm, alarm.getCustomerId(), - ActionType.ALARM_DELETE, user, alarm.getId()); - return alarmSubscriptionService.deleteAlarm(tenantId, alarm.getId()); + public boolean delete(Alarm alarm, User user) { + var tenantId = alarm.getTenantId(); + var alarmId = alarm.getId(); + var alarmOriginator = alarm.getOriginator(); + + boolean deleted; + try { + deleted = alarmSubscriptionService.deleteAlarm(tenantId, alarmId); + } catch (Exception e) { + logEntityActionService.logEntityAction(tenantId, emptyId(alarmOriginator.getEntityType()), ActionType.ALARM_DELETE, user, e, alarmId); + throw e; + } + + if (deleted) { + logEntityActionService.logEntityAction(tenantId, alarmOriginator, alarm, alarm.getCustomerId(), ActionType.ALARM_DELETE, user, alarmId); + } + + return deleted; } private static long getOrDefault(long ts) { diff --git a/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/TbAlarmService.java b/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/TbAlarmService.java index 11b5c864ac..c034c39561 100644 --- a/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/TbAlarmService.java +++ b/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/TbAlarmService.java @@ -43,5 +43,6 @@ public interface TbAlarmService { void unassignDeletedUserAlarms(TenantId tenantId, UserId userId, String userTitle, List alarms, long unassignTs); - Boolean delete(Alarm alarm, User user); + boolean delete(Alarm alarm, User user); + } diff --git a/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultAlarmSubscriptionService.java b/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultAlarmSubscriptionService.java index 7a5a563e68..fd9d8b7141 100644 --- a/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultAlarmSubscriptionService.java +++ b/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultAlarmSubscriptionService.java @@ -115,7 +115,7 @@ public class DefaultAlarmSubscriptionService extends AbstractSubscriptionService } @Override - public Boolean deleteAlarm(TenantId tenantId, AlarmId alarmId) { + public boolean deleteAlarm(TenantId tenantId, AlarmId alarmId) { AlarmApiCallResult result = alarmService.delAlarm(tenantId, alarmId); onAlarmDeleted(result); return result.isSuccessful(); diff --git a/application/src/test/java/org/thingsboard/server/controller/AlarmControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AlarmControllerTest.java index d80c8a6ba5..8a49d34d20 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AlarmControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AlarmControllerTest.java @@ -15,6 +15,7 @@ */ package org.thingsboard.server.controller; +import com.datastax.oss.driver.api.core.uuid.Uuids; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import lombok.extern.slf4j.Slf4j; @@ -57,6 +58,8 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.verifyNoInteractions; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @Slf4j @@ -308,6 +311,21 @@ public class AlarmControllerTest extends AbstractControllerTest { testNotifyEntityNever(alarm.getId(), alarm); } + @Test + public void testDeleteNonExistentAlarm() throws Exception { + loginTenantAdmin(); + + var nonExistentAlarmId = Uuids.timeBased(); + + Mockito.reset(tbClusterService, auditLogService); + + doDelete("/api/alarm/" + nonExistentAlarmId) + .andExpect(status().isNotFound()) + .andExpect(statusReason(is("Alarm with id [" + nonExistentAlarmId + "] is not found"))); + + verifyNoInteractions(tbClusterService, auditLogService); + } + @Test public void testClearAlarmViaCustomer() throws Exception { loginCustomerUser(); @@ -634,12 +652,12 @@ public class AlarmControllerTest extends AbstractControllerTest { doDelete("/api/user/" + savedUser.getId().getId()).andExpect(status().isOk()); - Awaitility.await().atMost(TIMEOUT, TimeUnit.SECONDS).untilAsserted(() -> { - AlarmInfo alarmInfo = doGet("/api/alarm/info/" + alarmId.getId(), AlarmInfo.class); - Assert.assertNotNull(alarmInfo); - Assert.assertNull(alarmInfo.getAssigneeId()); - Assert.assertTrue(alarmInfo.getAssignTs() >= afterAssignmentTs); - }); + Awaitility.await().atMost(TIMEOUT, TimeUnit.SECONDS).untilAsserted(() -> { + AlarmInfo alarmInfo = doGet("/api/alarm/info/" + alarmId.getId(), AlarmInfo.class); + Assert.assertNotNull(alarmInfo); + Assert.assertNull(alarmInfo.getAssigneeId()); + Assert.assertTrue(alarmInfo.getAssignTs() >= afterAssignmentTs); + }); } @Test diff --git a/application/src/test/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmServiceTest.java b/application/src/test/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmServiceTest.java index ac85b48bc1..670ab390ac 100644 --- a/application/src/test/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmServiceTest.java +++ b/application/src/test/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmServiceTest.java @@ -15,15 +15,12 @@ */ package org.thingsboard.server.service.entitiy.alarm; +import com.datastax.oss.driver.api.core.uuid.Uuids; import com.fasterxml.jackson.databind.node.ObjectNode; -import lombok.extern.slf4j.Slf4j; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.boot.test.mock.mockito.SpyBean; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.TestPropertySource; -import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.cluster.TbClusterService; import org.thingsboard.server.common.data.User; @@ -35,6 +32,9 @@ import org.thingsboard.server.common.data.alarm.AlarmInfo; import org.thingsboard.server.common.data.audit.ActionType; import org.thingsboard.server.common.data.exception.ThingsboardException; import org.thingsboard.server.common.data.id.AlarmId; +import org.thingsboard.server.common.data.id.CustomerId; +import org.thingsboard.server.common.data.id.DeviceId; +import org.thingsboard.server.common.data.id.EntityId; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.dao.alarm.AlarmService; @@ -47,7 +47,6 @@ import org.thingsboard.server.dao.edge.EdgeService; import org.thingsboard.server.dao.entity.EntityService; import org.thingsboard.server.dao.tenant.TenantService; import org.thingsboard.server.service.entitiy.TbLogEntityActionService; -import org.thingsboard.server.service.executors.DbCallbackExecutorService; import org.thingsboard.server.service.security.permission.AccessControlService; import org.thingsboard.server.service.sync.vc.EntitiesVersionControlService; import org.thingsboard.server.service.telemetry.AlarmSubscriptionService; @@ -55,58 +54,57 @@ import org.thingsboard.server.service.telemetry.AlarmSubscriptionService; import java.util.List; import java.util.UUID; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -@Slf4j -@RunWith(SpringRunner.class) -@ContextConfiguration(classes = DefaultTbAlarmService.class) -@TestPropertySource(properties = { - "server.log_controller_error_stack_trace=false" -}) -public class DefaultTbAlarmServiceTest { +@SpringJUnitConfig(DefaultTbAlarmService.class) +class DefaultTbAlarmServiceTest { @MockBean - protected DbCallbackExecutorService dbExecutor; + TbLogEntityActionService logEntityActionService; @MockBean - protected TbLogEntityActionService logEntityActionService; + EdgeService edgeService; @MockBean - protected EdgeService edgeService; + AlarmService alarmService; @MockBean - protected AlarmService alarmService; + TbAlarmCommentService alarmCommentService; @MockBean - protected TbAlarmCommentService alarmCommentService; + AlarmSubscriptionService alarmSubscriptionService; @MockBean - protected AlarmSubscriptionService alarmSubscriptionService; + CustomerService customerService; @MockBean - protected CustomerService customerService; + TbClusterService tbClusterService; @MockBean - protected TbClusterService tbClusterService; + EntitiesVersionControlService vcService; @MockBean - private EntitiesVersionControlService vcService; + AccessControlService accessControlService; @MockBean - private AccessControlService accessControlService; + TenantService tenantService; @MockBean - private TenantService tenantService; + AssetService assetService; @MockBean - private AssetService assetService; + DeviceService deviceService; @MockBean - private DeviceService deviceService; + AssetProfileService assetProfileService; @MockBean - private AssetProfileService assetProfileService; + DeviceProfileService deviceProfileService; @MockBean - private DeviceProfileService deviceProfileService; - @MockBean - private EntityService entityService; - @SpyBean + EntityService entityService; + + @Autowired DefaultTbAlarmService service; + TenantId tenantId = TenantId.fromUUID(Uuids.timeBased()); + CustomerId customerId = new CustomerId(Uuids.timeBased()); + @Test - public void testSave() throws ThingsboardException { + void testSave() throws ThingsboardException { var alarm = new AlarmInfo(); when(alarmSubscriptionService.createAlarm(any())).thenReturn(AlarmApiCallResult.builder() .successful(true) @@ -115,45 +113,99 @@ public class DefaultTbAlarmServiceTest { .build()); service.save(alarm, new User()); - verify(logEntityActionService, times(1)).logEntityAction(any(), any(), any(), any(), eq(ActionType.ADDED), any()); - verify(alarmSubscriptionService, times(1)).createAlarm(any()); + verify(logEntityActionService).logEntityAction(any(), any(), any(), any(), eq(ActionType.ADDED), any()); + verify(alarmSubscriptionService).createAlarm(any()); } @Test - public void testAck() throws ThingsboardException { + void testAck() throws ThingsboardException { var alarm = new Alarm(); when(alarmSubscriptionService.acknowledgeAlarm(any(), any(), anyLong())) .thenReturn(AlarmApiCallResult.builder().successful(true).modified(true).alarm(new AlarmInfo()).build()); service.ack(alarm, new User(new UserId(UUID.randomUUID()))); - verify(alarmCommentService, times(1)).saveAlarmComment(any(), any(), any()); - verify(logEntityActionService, times(1)).logEntityAction(any(), any(), any(), any(), eq(ActionType.ALARM_ACK), any()); - verify(alarmSubscriptionService, times(1)).acknowledgeAlarm(any(), any(), anyLong()); + verify(alarmCommentService).saveAlarmComment(any(), any(), any()); + verify(logEntityActionService).logEntityAction(any(), any(), any(), any(), eq(ActionType.ALARM_ACK), any()); + verify(alarmSubscriptionService).acknowledgeAlarm(any(), any(), anyLong()); } @Test - public void testClear() throws ThingsboardException { + void testClear() throws ThingsboardException { var alarm = new Alarm(); alarm.setAcknowledged(true); when(alarmSubscriptionService.clearAlarm(any(), any(), anyLong(), any())) .thenReturn(AlarmApiCallResult.builder().successful(true).cleared(true).alarm(new AlarmInfo()).build()); service.clear(alarm, new User(new UserId(UUID.randomUUID()))); - verify(alarmCommentService, times(1)).saveAlarmComment(any(), any(), any()); - verify(logEntityActionService, times(1)).logEntityAction(any(), any(), any(), any(), eq(ActionType.ALARM_CLEAR), any()); - verify(alarmSubscriptionService, times(1)).clearAlarm(any(), any(), anyLong(), any()); + verify(alarmCommentService).saveAlarmComment(any(), any(), any()); + verify(logEntityActionService).logEntityAction(any(), any(), any(), any(), eq(ActionType.ALARM_CLEAR), any()); + verify(alarmSubscriptionService).clearAlarm(any(), any(), anyLong(), any()); } @Test - public void testDelete() { - service.delete(new Alarm(), new User()); + void testDelete_deleteApiReturnsTrue_shouldLogActionAndReturnTrue() { + // GIVEN + var alarmOriginator = new DeviceId(Uuids.timeBased()); + + var alarm = new Alarm(new AlarmId(Uuids.timeBased())); + alarm.setTenantId(tenantId); + alarm.setCustomerId(customerId); + alarm.setOriginator(alarmOriginator); + + var user = new User(); + + when(alarmSubscriptionService.deleteAlarm(tenantId, alarm.getId())).thenReturn(true); - verify(logEntityActionService, times(1)).logEntityAction(any(), any(), any(), any(), eq(ActionType.ALARM_DELETE), any(), any()); - verify(alarmSubscriptionService, times(1)).deleteAlarm(any(), any()); + // WHEN + boolean actual = service.delete(alarm, user); + + assertThat(actual).isTrue(); + verify(logEntityActionService).logEntityAction(tenantId, alarmOriginator, alarm, alarm.getCustomerId(), ActionType.ALARM_DELETE, user, alarm.getId()); + verify(alarmSubscriptionService).deleteAlarm(tenantId, alarm.getId()); } @Test - public void testUnassignAlarm() throws ThingsboardException { + void testDelete_deleteApiReturnsFalse_shouldNotLogActionAndReturnFalse() { + // GIVEN + var alarm = new Alarm(new AlarmId(Uuids.timeBased())); + alarm.setTenantId(tenantId); + + var user = new User(); + + // WHEN + boolean actual = service.delete(alarm, user); + + assertThat(actual).isFalse(); + verifyNoInteractions(logEntityActionService); + verify(alarmSubscriptionService).deleteAlarm(tenantId, alarm.getId()); + } + + @Test + void testDelete_deleteApiThrowsException_shouldLogFailedActionAndRethrow() { + // GIVEN + var alarmOriginator = new DeviceId(Uuids.timeBased()); + + var alarm = new Alarm(new AlarmId(Uuids.timeBased())); + alarm.setTenantId(tenantId); + alarm.setOriginator(alarmOriginator); + + var user = new User(); + + var exception = new RuntimeException("failed to delete alarm"); + + when(alarmSubscriptionService.deleteAlarm(tenantId, alarm.getId())).thenThrow(exception); + + // WHEN-THEN + assertThatThrownBy(() -> service.delete(alarm, user)) + .isInstanceOf(RuntimeException.class) + .hasMessage("failed to delete alarm"); + + verify(logEntityActionService).logEntityAction(tenantId, new DeviceId(EntityId.NULL_UUID), ActionType.ALARM_DELETE, user, exception, alarm.getId()); + verify(alarmSubscriptionService).deleteAlarm(tenantId, alarm.getId()); + } + + @Test + void testUnassignAlarm() throws ThingsboardException { AlarmInfo alarm = new AlarmInfo(); alarm.setId(new AlarmId(UUID.randomUUID())); when(alarmSubscriptionService.unassignAlarm(any(), any(), anyLong())) @@ -174,12 +226,11 @@ public class DefaultTbAlarmServiceTest { .comment(commentNode) .build(); - verify(alarmCommentService, times(1)) - .saveAlarmComment(eq(alarm), eq(expectedAlarmComment), eq(user)); + verify(alarmCommentService).saveAlarmComment(eq(alarm), eq(expectedAlarmComment), eq(user)); } @Test - public void testUnassignDeletedUserAlarms() throws ThingsboardException { + void testUnassignDeletedUserAlarms() throws ThingsboardException { AlarmInfo alarm = new AlarmInfo(); alarm.setId(new AlarmId(UUID.randomUUID())); @@ -189,7 +240,7 @@ public class DefaultTbAlarmServiceTest { User user = new User(); user.setEmail("testEmail@gmail.com"); user.setId(new UserId(UUID.randomUUID())); - service.unassignDeletedUserAlarms(new TenantId(UUID.randomUUID()), user.getId(), user.getTitle(), List.of(alarm.getUuidId()), System.currentTimeMillis()); + service.unassignDeletedUserAlarms(tenantId, user.getId(), user.getTitle(), List.of(alarm.getUuidId()), System.currentTimeMillis()); ObjectNode commentNode = JacksonUtil.newObjectNode(); commentNode.put("subtype", "ASSIGN"); @@ -200,9 +251,7 @@ public class DefaultTbAlarmServiceTest { .comment(commentNode) .build(); - verify(alarmCommentService, times(1)) - .saveAlarmComment(eq(alarm), eq(expectedAlarmComment), eq(null)); + verify(alarmCommentService).saveAlarmComment(eq(alarm), eq(expectedAlarmComment), eq(null)); } - } diff --git a/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/RuleEngineAlarmService.java b/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/RuleEngineAlarmService.java index 20f2342ebe..48fda3b781 100644 --- a/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/RuleEngineAlarmService.java +++ b/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/RuleEngineAlarmService.java @@ -70,7 +70,7 @@ public interface RuleEngineAlarmService { AlarmApiCallResult unassignAlarm(TenantId tenantId, AlarmId alarmId, long assignTs); // Other API - Boolean deleteAlarm(TenantId tenantId, AlarmId alarmId); + boolean deleteAlarm(TenantId tenantId, AlarmId alarmId); ListenableFuture findAlarmByIdAsync(TenantId tenantId, AlarmId alarmId); @@ -99,4 +99,5 @@ public interface RuleEngineAlarmService { PageData findAlarmDataByQueryForEntities(TenantId tenantId, AlarmDataQuery query, Collection orderedEntityIds); PageData findAlarmTypesByTenantId(TenantId tenantId, PageLink pageLink); + }