From 5e033ff066c906031a812e4332bf875422285fcd Mon Sep 17 00:00:00 2001 From: IrynaMatveieva Date: Thu, 22 Aug 2024 14:04:33 +0300 Subject: [PATCH] added additional verification in tests --- .../server/common/data/audit/ActionType.java | 44 ++++++++++--------- .../common/data/audit/ActionTypeTest.java | 21 +++++++-- .../server/dao/audit/AuditLogServiceImpl.java | 35 +++++++++------ .../dao/audit/AuditLogServiceImplTest.java | 41 +++++++++-------- 4 files changed, 86 insertions(+), 55 deletions(-) diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/audit/ActionType.java b/common/data/src/main/java/org/thingsboard/server/common/data/audit/ActionType.java index 3a70dacf65..72e5afa558 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/audit/ActionType.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/audit/ActionType.java @@ -29,26 +29,26 @@ public enum ActionType { ATTRIBUTES_DELETED(TbMsgType.ATTRIBUTES_DELETED), // log attributes TIMESERIES_UPDATED(TbMsgType.TIMESERIES_UPDATED), // log timeseries update TIMESERIES_DELETED(TbMsgType.TIMESERIES_DELETED), // log timeseries - RPC_CALL(null), // log method and params - CREDENTIALS_UPDATED(null), // log new credentials + RPC_CALL(), // log method and params + CREDENTIALS_UPDATED(), // log new credentials ASSIGNED_TO_CUSTOMER(TbMsgType.ENTITY_ASSIGNED), // log customer name UNASSIGNED_FROM_CUSTOMER(TbMsgType.ENTITY_UNASSIGNED), // log customer name - ACTIVATED(null), // log string id - SUSPENDED(null), // log string id + ACTIVATED(), // log string id + SUSPENDED(), // log string id CREDENTIALS_READ(true), // log device id ATTRIBUTES_READ(true), // log attributes RELATION_ADD_OR_UPDATE(TbMsgType.RELATION_ADD_OR_UPDATE), RELATION_DELETED(TbMsgType.RELATION_DELETED), RELATIONS_DELETED(TbMsgType.RELATIONS_DELETED), - REST_API_RULE_ENGINE_CALL(null), // log call to rule engine from REST API + REST_API_RULE_ENGINE_CALL(), // log call to rule engine from REST API ALARM_ACK(TbMsgType.ALARM_ACK, true), ALARM_CLEAR(TbMsgType.ALARM_CLEAR, true), ALARM_DELETE(TbMsgType.ALARM_DELETE, true), ALARM_ASSIGNED(TbMsgType.ALARM_ASSIGNED, true), ALARM_UNASSIGNED(TbMsgType.ALARM_UNASSIGNED, true), - LOGIN(null), - LOGOUT(null), - LOCKOUT(null), + LOGIN(), + LOGOUT(), + LOCKOUT(), ASSIGNED_FROM_TENANT(TbMsgType.ENTITY_ASSIGNED_FROM_TENANT), ASSIGNED_TO_TENANT(TbMsgType.ENTITY_ASSIGNED_TO_TENANT), PROVISION_SUCCESS(TbMsgType.PROVISION_SUCCESS), @@ -57,33 +57,37 @@ public enum ActionType { UNASSIGNED_FROM_EDGE(TbMsgType.ENTITY_UNASSIGNED_FROM_EDGE), ADDED_COMMENT(TbMsgType.COMMENT_CREATED), UPDATED_COMMENT(TbMsgType.COMMENT_UPDATED), - DELETED_COMMENT(null), - SMS_SENT(null); + DELETED_COMMENT(), + SMS_SENT(); @Getter - private final boolean isRead; + private final boolean read; private final TbMsgType ruleEngineMsgType; @Getter private final boolean alarmAction; - ActionType(boolean isRead) { - this.isRead = isRead; - this.ruleEngineMsgType = null; - this.alarmAction = false; + ActionType() { + this(false, null, false); + } + + ActionType(boolean read) { + this(read, null, false); } ActionType(TbMsgType ruleEngineMsgType) { - this.isRead = false; - this.ruleEngineMsgType = ruleEngineMsgType; - this.alarmAction = false; + this(false, ruleEngineMsgType, false); } ActionType(TbMsgType ruleEngineMsgType, boolean isAlarmAction) { - this.isRead = false; + this(false, ruleEngineMsgType, isAlarmAction); + } + + ActionType(boolean read, TbMsgType ruleEngineMsgType, boolean alarmAction) { + this.read = read; this.ruleEngineMsgType = ruleEngineMsgType; - this.alarmAction = isAlarmAction; + this.alarmAction = alarmAction; } public Optional getRuleEngineMsgType() { diff --git a/common/data/src/test/java/org/thingsboard/server/common/data/audit/ActionTypeTest.java b/common/data/src/test/java/org/thingsboard/server/common/data/audit/ActionTypeTest.java index ddfe068937..7006d36b54 100644 --- a/common/data/src/test/java/org/thingsboard/server/common/data/audit/ActionTypeTest.java +++ b/common/data/src/test/java/org/thingsboard/server/common/data/audit/ActionTypeTest.java @@ -17,7 +17,8 @@ package org.thingsboard.server.common.data.audit; import org.junit.jupiter.api.Test; -import java.util.List; +import java.util.EnumSet; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.thingsboard.server.common.data.audit.ActionType.ACTIVATED; @@ -40,7 +41,7 @@ import static org.thingsboard.server.common.data.audit.ActionType.SUSPENDED; class ActionTypeTest { - private final List typesWithNullRuleEngineMsgType = List.of( + private final Set typesWithNullRuleEngineMsgType = EnumSet.of( RPC_CALL, CREDENTIALS_UPDATED, ACTIVATED, @@ -55,10 +56,12 @@ class ActionTypeTest { REST_API_RULE_ENGINE_CALL ); - private final List alarmActionTypes = List.of( + private final Set alarmActionTypes = EnumSet.of( ALARM_ACK, ALARM_CLEAR, ALARM_DELETE, ALARM_ASSIGNED, ALARM_UNASSIGNED ); + private final Set readActionTypes = EnumSet.of(CREDENTIALS_READ, ATTRIBUTES_READ); + // backward-compatibility tests @Test @@ -85,4 +88,16 @@ class ActionTypeTest { } } + @Test + void isReadActionTypeTest() { + var types = ActionType.values(); + for (var type : types) { + if (readActionTypes.contains(type)) { + assertThat(type.isRead()).isTrue(); + } else { + assertThat(type.isRead()).isFalse(); + } + } + } + } diff --git a/dao/src/main/java/org/thingsboard/server/dao/audit/AuditLogServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/audit/AuditLogServiceImpl.java index 933909e7e9..b0c6e506b3 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/audit/AuditLogServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/audit/AuditLogServiceImpl.java @@ -123,19 +123,7 @@ public class AuditLogServiceImpl implements AuditLogService { JsonNode actionData = constructActionData(entityId, entity, actionType, additionalInfo); ActionStatus actionStatus = ActionStatus.SUCCESS; String failureDetails = ""; - String entityName = "N/A"; - if (entity != null) { - if (actionType.isAlarmAction()) { - entityName = (entity instanceof AlarmInfo alarmInfo) - ? alarmInfo.getOriginatorName() - : entityService.fetchEntityName(tenantId, entityId).orElse(entityName); - } else { - entityName = entity.getName(); - } - } else try { - entityName = entityService.fetchEntityName(tenantId, entityId).orElse(entityName); - } catch (Exception ignored) { - } + String entityName = getEntityName(tenantId, entityId, entity, actionType); if (e != null) { actionStatus = ActionStatus.FAILURE; failureDetails = getFailureStack(e); @@ -162,6 +150,27 @@ public class AuditLogServiceImpl implements AuditLogService { } } + private String getEntityName(TenantId tenantId, I entityId, E entity, ActionType actionType) { + if (entity == null) { + return fetchEntityName(tenantId, entityId); + } + if (!actionType.isAlarmAction()) { + return entity.getName(); + } + if (entity instanceof AlarmInfo alarmInfo) { + return alarmInfo.getOriginatorName(); + } + return fetchEntityName(tenantId, entityId); + } + + private String fetchEntityName(TenantId tenantId, I entityId) { + try { + return entityService.fetchEntityName(tenantId, entityId).orElse("N/A"); + } catch (Exception ignored) { + return "N/A"; + } + } + private JsonNode constructActionData(I entityId, E entity, ActionType actionType, Object... additionalInfo) { diff --git a/dao/src/test/java/org/thingsboard/server/dao/audit/AuditLogServiceImplTest.java b/dao/src/test/java/org/thingsboard/server/dao/audit/AuditLogServiceImplTest.java index 2a876c58db..730fc0e458 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/audit/AuditLogServiceImplTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/audit/AuditLogServiceImplTest.java @@ -37,6 +37,7 @@ import org.thingsboard.server.dao.entity.EntityService; import org.thingsboard.server.dao.service.validator.AuditLogDataValidator; import org.thingsboard.server.dao.sql.JpaExecutorService; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.Callable; @@ -44,7 +45,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; -import static org.mockito.BDDMockito.never; import static org.mockito.BDDMockito.then; @ExtendWith(MockitoExtension.class) @@ -73,15 +73,17 @@ public class AuditLogServiceImplTest { private AuditLogSink auditLogSink; @Test - public void givenEntityIsNull_whenLogEntityAction_thenShouldFetchEntityName() { + public void givenEntityIsNull_whenLogEntityAction_thenShouldFetchEntityName() throws Exception { // GIVEN given(auditLogLevelFilter.logEnabled(any(), any())).willReturn(true); + given(entityService.fetchEntityName(any(), any())).willReturn(Optional.of("Test device")); // WHEN auditLogService.logEntityAction(TENANT_ID, CUSTOMER_ID, USER_ID, USER_NAME, DEVICE_ID, null, ActionType.ADDED, null); // THEN then(entityService).should().fetchEntityName(TENANT_ID, DEVICE_ID); + verifyEntityName("Test device"); } @Test @@ -89,7 +91,7 @@ public class AuditLogServiceImplTest { // GIVEN given(auditLogLevelFilter.logEnabled(any(), any())).willReturn(true); Alarm alarm = new Alarm(new AlarmId(UUID.fromString("55f577b3-6ef5-4b99-92dc-70eb78b2a970"))); - alarm.setType("Test Alarm"); + alarm.setType("Test alarm"); AlarmComment comment = new AlarmComment(); comment.setComment(JacksonUtil.toJsonNode("{\"comment\": \"test\"}")); @@ -97,13 +99,7 @@ public class AuditLogServiceImplTest { auditLogService.logEntityAction(TENANT_ID, CUSTOMER_ID, USER_ID, USER_NAME, alarm.getId(), alarm, ActionType.ADDED_COMMENT, null, comment); // THEN - then(entityService).should(never()).fetchEntityName(any(), any()); - ArgumentCaptor submitTask = ArgumentCaptor.forClass(Callable.class); - then(executor).should().submit(submitTask.capture()); - submitTask.getValue().call(); - ArgumentCaptor auditLogEntry = ArgumentCaptor.forClass(AuditLog.class); - then(auditLogDao).should().save(eq(TENANT_ID), auditLogEntry.capture()); - assertThat(auditLogEntry.getValue().getEntityName()).isEqualTo("Test Alarm"); + verifyEntityName("Test alarm"); } @Test @@ -111,31 +107,38 @@ public class AuditLogServiceImplTest { // GIVEN given(auditLogLevelFilter.logEnabled(any(), any())).willReturn(true); AlarmInfo alarmInfo = new AlarmInfo(); - alarmInfo.setOriginatorName("Test Device"); + alarmInfo.setOriginatorName("Test device"); // WHEN auditLogService.logEntityAction(TENANT_ID, CUSTOMER_ID, USER_ID, USER_NAME, DEVICE_ID, alarmInfo, ActionType.ALARM_ASSIGNED, null); // THEN - then(entityService).should(never()).fetchEntityName(any(), any()); - ArgumentCaptor submitTask = ArgumentCaptor.forClass(Callable.class); - then(executor).should().submit(submitTask.capture()); - submitTask.getValue().call(); - ArgumentCaptor auditLogEntry = ArgumentCaptor.forClass(AuditLog.class); - then(auditLogDao).should().save(eq(TENANT_ID), auditLogEntry.capture()); - assertThat(auditLogEntry.getValue().getEntityName()).isEqualTo("Test Device"); + verifyEntityName("Test device"); } @Test - public void givenActionTypeIsAlarmActionAndEntityIsAlarm_whenLogEntityAction_thenShouldFetchEntityName() { + public void givenActionTypeIsAlarmActionAndEntityIsAlarm_whenLogEntityAction_thenShouldFetchEntityName() throws Exception { // GIVEN given(auditLogLevelFilter.logEnabled(any(), any())).willReturn(true); + given(entityService.fetchEntityName(any(), any())).willReturn(Optional.of("Test alarm")); // WHEN auditLogService.logEntityAction(TENANT_ID, CUSTOMER_ID, USER_ID, USER_NAME, DEVICE_ID, new Alarm(), ActionType.ALARM_DELETE, null); // THEN then(entityService).should().fetchEntityName(TENANT_ID, DEVICE_ID); + verifyEntityName("Test alarm"); + } + + private void verifyEntityName(String entityName) throws Exception { + then(auditLogDataValidator).should().validate(any(AuditLog.class), any()); + ArgumentCaptor submitTask = ArgumentCaptor.forClass(Callable.class); + then(executor).should().submit(submitTask.capture()); + submitTask.getValue().call(); + ArgumentCaptor auditLogEntry = ArgumentCaptor.forClass(AuditLog.class); + then(auditLogDao).should().save(eq(TENANT_ID), auditLogEntry.capture()); + assertThat(auditLogEntry.getValue().getEntityName()).isEqualTo(entityName); + then(auditLogSink).should().logAction(any()); } }