Code review fixes

Volodymyr Babak
1 parent fad04b0d
Showing 6 changed files with 93 additions and 39 deletions
application/src/main/java/org/thingsboard/server/controller/BaseController.java
application/src/main/java/org/thingsboard/server/controller/DeviceController.java
application/src/main/resources/thingsboard.yml
dao/src/main/java/org/thingsboard/server/dao/audit/CassandraAuditLogDao.java
dao/src/main/java/org/thingsboard/server/dao/audit/DummyAuditLogServiceImpl.java
dao/src/test/resources/application-test.properties
--- a/application/src/main/java/org/thingsboard/server/controller/BaseController.java
View file @92fdbbf
+++ b/application/src/main/java/org/thingsboard/server/controller/BaseController.java
View file @92fdbbf
@@ -15,9 +15,12 @@
  */
 package org.thingsboard.server.controller;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.web.bind.annotation.ExceptionHandler;
@@ -27,6 +30,8 @@ import org.thingsboard.server.common.data.alarm.Alarm;
 import org.thingsboard.server.common.data.alarm.AlarmId;
 import org.thingsboard.server.common.data.alarm.AlarmInfo;
 import org.thingsboard.server.common.data.asset.Asset;
+import org.thingsboard.server.common.data.audit.ActionStatus;
+import org.thingsboard.server.common.data.audit.ActionType;
 import org.thingsboard.server.common.data.id.*;
 import org.thingsboard.server.common.data.page.TextPageLink;
 import org.thingsboard.server.common.data.page.TimePageLink;
@@ -73,6 +78,10 @@ public abstract class BaseController {
     public static final String INCORRECT_TENANT_ID = "Incorrect tenantId ";
     public static final String YOU_DON_T_HAVE_PERMISSION_TO_PERFORM_THIS_OPERATION = "You don't have permission to perform this operation!";
+
+    @Value("${audit_log.exceptions.enabled}")
+    private boolean auditLogExceptionsEnabled;
+
     @Autowired
     private ThingsboardErrorResponseHandler errorResponseHandler;
@@ -131,6 +140,11 @@ public abstract class BaseController {
         return handleException(exception, true);
     }
+    ThingsboardException handleException(Exception exception, ActionType actionType, String actionData) {
+        logExceptionToAuditLog(exception, actionType, actionData);
+        return handleException(exception, true);
+    }
+
     private ThingsboardException handleException(Exception exception, boolean logException) {
         if (logException) {
             log.error("Error [{}]", exception.getMessage());
@@ -153,6 +167,36 @@ public abstract class BaseController {
         }
     }
+    private void logExceptionToAuditLog(Exception exception, ActionType actionType, String actionData) {
+        try {
+            if (auditLogExceptionsEnabled) {
+                SecurityUser currentUser = getCurrentUser();
+                EntityId entityId;
+                CustomerId customerId;
+                if (!currentUser.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) {
+                    entityId = currentUser.getCustomerId();
+                    customerId = currentUser.getCustomerId();
+                } else {
+                    entityId = currentUser.getTenantId();
+                    customerId = new CustomerId(ModelConstants.NULL_UUID);
+                }
+
+                JsonNode actionDataNode = new ObjectMapper().createObjectNode().put("actionData", actionData);
+
+                auditLogService.logEntityAction(currentUser,
+                        entityId,
+                        null,
+                        customerId,
+                        actionType,
+                        actionDataNode,
+                        ActionStatus.FAILURE,
+                        exception.getMessage());
+            }
+        } catch (Exception e) {
+            log.error("Exception happend during saving to audit log", e);
+        }
+    }
+
     <T> T checkNotNull(T reference) throws ThingsboardException {
         if (reference == null) {
             throw new ThingsboardException("Requested item wasn't found!", ThingsboardErrorCode.ITEM_NOT_FOUND);
@@ -545,4 +589,24 @@ public abstract class BaseController {
                 serverPort);
         return baseUrl;
     }
+
+    protected void logEntityDeleted(EntityId entityId, String entityName, CustomerId customerId) throws ThingsboardException {
+        logEntitySuccess(entityId, entityName, customerId, ActionType.DELETED);
+    }
+
+    protected void logEntityAddedOrUpdated(EntityId entityId, String entityName, CustomerId customerId, boolean isAddAction) throws ThingsboardException {
+        logEntitySuccess(entityId, entityName, customerId, isAddAction ? ActionType.ADDED : ActionType.UPDATED);
+    }
+
+    protected void logEntitySuccess(EntityId entityId, String entityName, CustomerId customerId, ActionType actionType) throws ThingsboardException {
+        auditLogService.logEntityAction(
+                getCurrentUser(),
+                entityId,
+                entityName,
+                customerId,
+                actionType,
+                null,
+                ActionStatus.SUCCESS,
+                null);
+    }
 }
--- a/application/src/main/java/org/thingsboard/server/controller/DeviceController.java
View file @92fdbbf
+++ b/application/src/main/java/org/thingsboard/server/controller/DeviceController.java
View file @92fdbbf
@@ -85,20 +85,11 @@ public class DeviceController extends BaseController {
                             savedDevice.getName(),
                             savedDevice.getType());
-            auditLogService.logEntityAction(
-                    getCurrentUser(),
-                    savedDevice.getId(),
-                    savedDevice.getName(),
-                    savedDevice.getCustomerId(),
-                    device.getId() == null ? ActionType.ADDED : ActionType.UPDATED,
-                    null,
-                    ActionStatus.SUCCESS,
-                    null);
-
+            logEntityAddedOrUpdated(savedDevice.getId(), savedDevice.getName(), savedDevice.getCustomerId(), device.getId() == null);
             return savedDevice;
         } catch (Exception e) {
-            throw handleException(e);
+            throw handleException(e, device.getId() == null ? ActionType.ADDED : ActionType.UPDATED, "addDevice(" + device + ")");
         }
     }
@@ -111,17 +102,9 @@ public class DeviceController extends BaseController {
             DeviceId deviceId = new DeviceId(toUUID(strDeviceId));
             Device device = checkDeviceId(deviceId);
             deviceService.deleteDevice(deviceId);
-            auditLogService.logEntityAction(
-                    getCurrentUser(),
-                    device.getId(),
-                    device.getName(),
-                    device.getCustomerId(),
-                    ActionType.DELETED,
-                    null,
-                    ActionStatus.SUCCESS,
-                    null);
+            logEntityDeleted(device.getId(), device.getName(), device.getCustomerId());
         } catch (Exception e) {
-            throw handleException(e);
+            throw handleException(e, ActionType.DELETED, "deleteDevice(" + strDeviceId + ")");
         }
     }
@@ -200,18 +183,10 @@ public class DeviceController extends BaseController {
             Device device = checkDeviceId(deviceCredentials.getDeviceId());
             DeviceCredentials result = checkNotNull(deviceCredentialsService.updateDeviceCredentials(deviceCredentials));
             actorService.onCredentialsUpdate(getCurrentUser().getTenantId(), deviceCredentials.getDeviceId());
-            auditLogService.logEntityAction(
-                    getCurrentUser(),
-                    device.getId(),
-                    device.getName(),
-                    device.getCustomerId(),
-                    ActionType.CREDENTIALS_UPDATED,
-                    null,
-                    ActionStatus.SUCCESS,
-                    null);
+            logEntitySuccess(device.getId(), device.getName(), device.getCustomerId(), ActionType.CREDENTIALS_UPDATED);
             return result;
         } catch (Exception e) {
-            throw handleException(e);
+            throw handleException(e, ActionType.CREDENTIALS_UPDATED, "saveDeviceCredentials(" + deviceCredentials + ")");
         }
     }
--- a/application/src/main/resources/thingsboard.yml
View file @92fdbbf
+++ b/application/src/main/resources/thingsboard.yml
View file @92fdbbf
@@ -286,4 +286,11 @@ spring:
 # Audit log parameters
 audit_log:
   # Enable/disable audit log functionality.
-  enabled: "${AUDIT_LOG_ENABLED:true}"
+  enabled: "${AUDIT_LOG_ENABLED:true}"
+  # Specify partitioning size for audit log by tenant id storage. Example MINUTES, HOURS, DAYS, MONTHS
+  by_tenant_partitioning: "${AUDIT_LOG_BY_TENANT_PARTITIONING:MONTHS}"
+  # Number of days as history period if startTime and endTime are not specified
+  default_query_period: "${AUDIT_LOG_DEFAULT_QUERY_PERIOD:30}"
+  exceptions:
+    # Enable/disable audit log functionality for exceptions.
+    enabled: "${AUDIT_LOG_EXCEPTIONS_ENABLED:true}"
--- a/dao/src/main/java/org/thingsboard/server/dao/audit/CassandraAuditLogDao.java
View file @92fdbbf
+++ b/dao/src/main/java/org/thingsboard/server/dao/audit/CassandraAuditLogDao.java
View file @92fdbbf
@@ -81,10 +81,13 @@ public class CassandraAuditLogDao extends CassandraAbstractSearchTimeDao<AuditLo
     protected ExecutorService readResultsProcessingExecutor;
-    @Value("${cassandra.query.ts_key_value_partitioning}")
+    @Value("${audit_log.by_tenant_partitioning}")
     private String partitioning;
     private TsPartitionDate tsFormat;
+    @Value("${audit_log.default_query_period}")
+    private Integer defaultQueryPeriodInDays;
+
     private PreparedStatement partitionInsertStmt;
     private PreparedStatement saveByTenantStmt;
     private PreparedStatement saveByTenantIdAndUserIdStmt;
@@ -304,7 +307,7 @@ public class CassandraAuditLogDao extends CassandraAbstractSearchTimeDao<AuditLo
         if (pageLink.getStartTime() != null && pageLink.getStartTime() != 0) {
             minPartition = toPartitionTs(pageLink.getStartTime());
         } else {
-            minPartition = toPartitionTs(LocalDate.now().minusMonths(1).atStartOfDay().toInstant(ZoneOffset.UTC).toEpochMilli());
+            minPartition = toPartitionTs(LocalDate.now().minusDays(defaultQueryPeriodInDays).atStartOfDay().toInstant(ZoneOffset.UTC).toEpochMilli());
         }
         long maxPartition;
--- a/dao/src/main/java/org/thingsboard/server/dao/audit/DummyAuditLogServiceImpl.java
View file @92fdbbf
+++ b/dao/src/main/java/org/thingsboard/server/dao/audit/DummyAuditLogServiceImpl.java
View file @92fdbbf
@@ -16,6 +16,7 @@
 package org.thingsboard.server.dao.audit;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.thingsboard.server.common.data.User;
@@ -29,6 +30,7 @@ import org.thingsboard.server.common.data.id.UserId;
 import org.thingsboard.server.common.data.page.TimePageData;
 import org.thingsboard.server.common.data.page.TimePageLink;
+import java.util.Collections;
 import java.util.List;
 @ConditionalOnProperty(prefix = "audit_log", value = "enabled", havingValue = "false")
@@ -36,26 +38,26 @@ public class DummyAuditLogServiceImpl implements AuditLogService {
     @Override
     public TimePageData<AuditLog> findAuditLogsByTenantIdAndCustomerId(TenantId tenantId, CustomerId customerId, TimePageLink pageLink) {
-        return null;
+        return new TimePageData<>(null, pageLink);
     }
     @Override
     public TimePageData<AuditLog> findAuditLogsByTenantIdAndUserId(TenantId tenantId, UserId userId, TimePageLink pageLink) {
-        return null;
+        return new TimePageData<>(null, pageLink);
     }
     @Override
     public TimePageData<AuditLog> findAuditLogsByTenantIdAndEntityId(TenantId tenantId, EntityId entityId, TimePageLink pageLink) {
-        return null;
+        return new TimePageData<>(null, pageLink);
     }
     @Override
     public TimePageData<AuditLog> findAuditLogsByTenantId(TenantId tenantId, TimePageLink pageLink) {
-        return null;
+        return new TimePageData<>(null, pageLink);
     }
     @Override
     public ListenableFuture<List<Void>> logEntityAction(User user, EntityId entityId, String entityName, CustomerId customerId, ActionType actionType, JsonNode actionData, ActionStatus actionStatus, String actionFailureDetails) {
-        return null;
+        return Futures.immediateFuture(Collections.emptyList());
     }
 }
--- a/dao/src/test/resources/application-test.properties
View file @92fdbbf
+++ b/dao/src/test/resources/application-test.properties
View file @92fdbbf
@@ -10,6 +10,9 @@ zk.zk_dir=/thingsboard
 updates.enabled=false
 audit_log.enabled=true
+audit_log.exceptions.enabled=false
+audit_log.by_tenant_partitioning=MONTHS
+audit_log.default_query_period=30
 caching.specs.relations.timeToLiveInMinutes=1440
 caching.specs.relations.maxSize=100000