sysadmin audit log refactored after review

Signed-off-by: Oleksandra_Matviienko <al.zzzeebra@gmail.com>

application/src/main/java/org/thingsboard/server/controller/AuditLogController.java

@@ -50,6 +50,7 @@ import static org.thingsboard.server.controller.ControllerConstants.PAGE_DATA_PA
 import static org.thingsboard.server.controller.ControllerConstants.PAGE_NUMBER_DESCRIPTION;
 import static org.thingsboard.server.controller.ControllerConstants.PAGE_SIZE_DESCRIPTION;
 import static org.thingsboard.server.controller.ControllerConstants.SORT_ORDER_DESCRIPTION;
+import static org.thingsboard.server.controller.ControllerConstants.SYSTEM_OR_TENANT_AUTHORITY_PARAGRAPH;
 import static org.thingsboard.server.controller.ControllerConstants.TENANT_AUTHORITY_PARAGRAPH;
 import static org.thingsboard.server.controller.ControllerConstants.USER_ID_PARAM_DESCRIPTION;
 
@@ -138,7 +139,7 @@ public class AuditLogController extends BaseController {
             notes = "Returns a page of audit logs related to the actions on the targeted entity. " +
                     "Basically, this API call is used to get the full lifecycle of some specific entity. " +
                     "For example to see when a device was created, updated, assigned to some customer, or even deleted from the system. " +
-                    PAGE_DATA_PARAMETERS + TENANT_AUTHORITY_PARAGRAPH)
+                    PAGE_DATA_PARAMETERS + SYSTEM_OR_TENANT_AUTHORITY_PARAGRAPH)
     @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN')")
     @RequestMapping(value = "/audit/logs/entity/{entityType}/{entityId}", params = {"pageSize", "page"}, method = RequestMethod.GET)
     @ResponseBody
@@ -173,7 +174,7 @@ public class AuditLogController extends BaseController {
 
     @ApiOperation(value = "Get all audit logs (getAuditLogs)",
             notes = "Returns a page of audit logs related to all entities in the scope of the current user's Tenant. " +
-                    PAGE_DATA_PARAMETERS + TENANT_AUTHORITY_PARAGRAPH)
+                    PAGE_DATA_PARAMETERS + SYSTEM_OR_TENANT_AUTHORITY_PARAGRAPH)
     @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN')")
     @RequestMapping(value = "/audit/logs", params = {"pageSize", "page"}, method = RequestMethod.GET)
     @ResponseBody

application/src/main/java/org/thingsboard/server/controller/BaseController.java

@@ -881,10 +881,8 @@ public abstract class BaseController {
 
     protected <E extends HasName & HasId<? extends EntityId>> void logEntityAction(SecurityUser user, EntityType entityType, E entity, E savedEntity, ActionType actionType, Exception e) {
         EntityId entityId = savedEntity != null ? savedEntity.getId() : emptyId(entityType);
-        if (!user.isSystemAdmin()) {
+        entityActionService.logEntityAction(user, entityId, savedEntity != null ? savedEntity : entity,
-            entityActionService.logEntityAction(user, entityId, savedEntity != null ? savedEntity : entity,
+                user.getCustomerId(), actionType, e);
-                    user.getCustomerId(), actionType, e);
-        }
     }
 
     protected <E extends HasName & HasId<? extends EntityId>> E doSaveAndLog(EntityType entityType, E entity, BiFunction<TenantId, E, E> savingFunction) throws Exception {

application/src/main/java/org/thingsboard/server/service/entitiy/tenant/profile/DefaultTbTenantProfileService.java

@@ -52,11 +52,11 @@ public class DefaultTbTenantProfileService extends AbstractTbEntityService imple
         try {
             TenantProfile savedTenantProfile = checkNotNull(tenantProfileService.saveTenantProfile(tenantId, tenantProfile));
             tenantProfileCache.put(savedTenantProfile);
+            logEntityActionService.logEntityAction(tenantId, savedTenantProfile.getId(), savedTenantProfile, null,
+                    actionType, user);
 
             List<TenantId> tenantIds = tenantService.findTenantIdsByTenantProfileId(savedTenantProfile.getId());
             tbQueueService.updateQueuesByTenants(tenantIds, savedTenantProfile, oldTenantProfile);
-            logEntityActionService.logEntityAction(tenantId, savedTenantProfile.getId(), savedTenantProfile, null,
-                    actionType, user);
 
             return savedTenantProfile;
         } catch (ThingsboardException e) {
@@ -91,14 +91,9 @@ public class DefaultTbTenantProfileService extends AbstractTbEntityService imple
     public TenantProfile setDefaultTenantProfile(TenantId tenantId, TenantProfile tenantProfile, User user) throws ThingsboardException {
         ActionType actionType = ActionType.UPDATED;
         try {
-            boolean changed = tenantProfileService.setDefaultTenantProfile(tenantId, tenantProfile.getId());
+            TenantProfile savedTenantProfile = tenantProfileService.setDefaultTenantProfile(tenantId, tenantProfile.getId());
-            TenantProfile result = tenantProfileService.findTenantProfileById(tenantId, tenantProfile.getId());
+            logEntityActionService.logEntityAction(tenantId, tenantProfile.getId(), savedTenantProfile, null, actionType, user);
-            if (changed && result != null) {
+            return savedTenantProfile;
-                // Update application-level cache
-                tenantProfileCache.put(result);
-            }
-            logEntityActionService.logEntityAction(tenantId, result != null ? result.getId() : tenantProfile.getId(), result, null, actionType, user);
-            return result != null ? result : tenantProfile;
         } catch (DataValidationException e) {
             log.error("Failed to set default tenant profile due to data validation [{}]", tenantProfile, e);
             logEntityActionService.logEntityAction(tenantId, emptyId(EntityType.TENANT_PROFILE), tenantProfile, actionType, user, e);

application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java

@@ -1268,6 +1268,7 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest {
         TenantProfile oldTenantProfile = tenantProfileService.findDefaultTenantProfile(TenantId.SYS_TENANT_ID);
         TenantProfile tenantProfile = JacksonUtil.clone(oldTenantProfile);
         updater.accept(tenantProfile);
+        // user should be sysadmin as this operation allowed only for sysadmins. But for the simplification of the test - already existed variable provided. This affects only an audit log content
         tbTenantProfileService.save(TenantId.SYS_TENANT_ID, tenantProfile, oldTenantProfile, tenantAdminUser);
     }
 

application/src/test/java/org/thingsboard/server/controller/TenantProfileControllerTest.java

@@ -16,6 +16,7 @@
 package org.thingsboard.server.controller;
 
 import com.fasterxml.jackson.core.type.TypeReference;
+import org.awaitility.Awaitility;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.ArgumentMatcher;
@@ -25,12 +26,12 @@ import org.thingsboard.server.common.data.EntityInfo;
 import org.thingsboard.server.common.data.StringUtils;
 import org.thingsboard.server.common.data.Tenant;
 import org.thingsboard.server.common.data.TenantProfile;
+import org.thingsboard.server.common.data.audit.AuditLog;
 import org.thingsboard.server.common.data.audit.ActionType;
 import org.thingsboard.server.common.data.id.TenantProfileId;
 import org.thingsboard.server.common.data.page.PageData;
 import org.thingsboard.server.common.data.page.PageLink;
 import org.thingsboard.server.common.data.page.TimePageLink;
-import org.thingsboard.server.common.data.audit.AuditLog;
 import org.thingsboard.server.common.data.plugin.ComponentLifecycleEvent;
 import org.thingsboard.server.common.data.queue.ProcessingStrategy;
 import org.thingsboard.server.common.data.queue.ProcessingStrategyType;
@@ -42,7 +43,6 @@ import org.thingsboard.server.common.data.tenant.profile.TenantProfileQueueConfi
 import org.thingsboard.server.common.data.validation.RateLimit;
 import org.thingsboard.server.dao.service.DaoSqlTest;
 import org.thingsboard.server.queue.TbQueueCallback;
-import org.awaitility.Awaitility;
 
 import java.lang.reflect.Field;
 import java.util.ArrayList;
@@ -191,7 +191,8 @@ public class TenantProfileControllerTest extends AbstractControllerTest {
         Assert.assertEquals(savedTenantProfile.getName(), foundDefaultTenantProfile.getName());
         Assert.assertEquals(savedTenantProfile.getId(), foundDefaultTenantProfile.getId());
 
-        awaitAuditLog("Wait for async audit log to be persisted (UPDATED expected for NEW DEFAULT Tenant Profile)", savedTenantProfile.getId(), ActionType.UPDATED);    }
+        awaitAuditLog("Wait for async audit log to be persisted (UPDATED expected for NEW DEFAULT Tenant Profile)", savedTenantProfile.getId(), ActionType.UPDATED);
+    }
 
     @Test
     public void testSaveTenantProfileWithEmptyName() throws Exception {

common/dao-api/src/main/java/org/thingsboard/server/dao/tenant/TenantProfileService.java

@@ -46,7 +46,7 @@ public interface TenantProfileService extends EntityDaoService {
 
     EntityInfo findDefaultTenantProfileInfo(TenantId tenantId);
 
-    boolean setDefaultTenantProfile(TenantId tenantId, TenantProfileId tenantProfileId);
+    TenantProfile setDefaultTenantProfile(TenantId tenantId, TenantProfileId tenantProfileId);
 
     void deleteTenantProfiles(TenantId tenantId);
 

dao/src/main/java/org/thingsboard/server/dao/audit/AuditLogServiceImpl.java

@@ -119,7 +119,7 @@ public class AuditLogServiceImpl implements AuditLogService {
     public <E extends HasName, I extends EntityId> ListenableFuture<Void>
     logEntityAction(TenantId tenantId, CustomerId customerId, UserId userId, String userName, I entityId, E entity,
                     ActionType actionType, Exception e, Object... additionalInfo) {
-        if (canLog(entityId.getEntityType(), actionType) || tenantId.isSysTenantId()) {
+        if (canLog(entityId.getEntityType(), actionType) || (tenantId != null && tenantId.isSysTenantId())) {
             JsonNode actionData = constructActionData(entityId, entity, actionType, additionalInfo);
             ActionStatus actionStatus = ActionStatus.SUCCESS;
             String failureDetails = "";

dao/src/main/java/org/thingsboard/server/dao/tenant/TenantProfileServiceImpl.java

@@ -190,7 +190,7 @@ public class TenantProfileServiceImpl extends AbstractCachedEntityService<Tenant
     }
 
     @Override
-    public boolean setDefaultTenantProfile(TenantId tenantId, TenantProfileId tenantProfileId) {
+    public TenantProfile setDefaultTenantProfile(TenantId tenantId, TenantProfileId tenantProfileId) {
         log.trace("Executing setDefaultTenantProfile [{}]", tenantProfileId);
         validateId(tenantId, id -> INCORRECT_TENANT_ID + id);
         validateId(tenantProfileId, id -> INCORRECT_TENANT_PROFILE_ID + id);
@@ -198,22 +198,18 @@ public class TenantProfileServiceImpl extends AbstractCachedEntityService<Tenant
         if (!tenantProfile.isDefault()) {
             tenantProfile.setDefault(true);
             TenantProfile previousDefaultTenantProfile = findDefaultTenantProfile(tenantId);
-            boolean changed = false;
             if (previousDefaultTenantProfile == null) {
-                tenantProfileDao.save(tenantId, tenantProfile);
+                tenantProfile = tenantProfileDao.save(tenantId, tenantProfile);
                 publishEvictEvent(new TenantProfileEvictEvent(tenantProfileId, true));
-                changed = true;
             } else if (!previousDefaultTenantProfile.getId().equals(tenantProfile.getId())) {
                 previousDefaultTenantProfile.setDefault(false);
                 tenantProfileDao.save(tenantId, previousDefaultTenantProfile);
-                tenantProfileDao.save(tenantId, tenantProfile);
+                tenantProfile = tenantProfileDao.save(tenantId, tenantProfile);
                 publishEvictEvent(new TenantProfileEvictEvent(previousDefaultTenantProfile.getId(), false));
                 publishEvictEvent(new TenantProfileEvictEvent(tenantProfileId, true));
-                changed = true;
             }
-            return changed;
         }
-        return false;
+        return tenantProfile;
     }
 
     @Override

dao/src/test/java/org/thingsboard/server/dao/service/TenantProfileServiceTest.java

@@ -164,13 +164,13 @@ public class TenantProfileServiceTest extends AbstractServiceTest {
         TenantProfile savedTenantProfile1 = tenantProfileService.saveTenantProfile(TenantId.SYS_TENANT_ID, tenantProfile1);
         TenantProfile savedTenantProfile2 = tenantProfileService.saveTenantProfile(TenantId.SYS_TENANT_ID, tenantProfile2);
 
-        boolean result = tenantProfileService.setDefaultTenantProfile(TenantId.SYS_TENANT_ID, savedTenantProfile1.getId());
+        TenantProfile setDefaultTenantProfile1 = tenantProfileService.setDefaultTenantProfile(TenantId.SYS_TENANT_ID, savedTenantProfile1.getId());
-        Assert.assertTrue(result);
+        Assert.assertNotEquals(savedTenantProfile1.isDefault(), setDefaultTenantProfile1.isDefault());
         TenantProfile defaultTenantProfile = tenantProfileService.findDefaultTenantProfile(TenantId.SYS_TENANT_ID);
         Assert.assertNotNull(defaultTenantProfile);
         Assert.assertEquals(savedTenantProfile1.getId(), defaultTenantProfile.getId());
-        result = tenantProfileService.setDefaultTenantProfile(TenantId.SYS_TENANT_ID, savedTenantProfile2.getId());
+        TenantProfile setDefaultTenantProfile2 = tenantProfileService.setDefaultTenantProfile(TenantId.SYS_TENANT_ID, savedTenantProfile2.getId());
-        Assert.assertTrue(result);
+        Assert.assertNotEquals(savedTenantProfile2.isDefault(), setDefaultTenantProfile2.isDefault());
         defaultTenantProfile = tenantProfileService.findDefaultTenantProfile(TenantId.SYS_TENANT_ID);
         Assert.assertNotNull(defaultTenantProfile);
         Assert.assertEquals(savedTenantProfile2.getId(), defaultTenantProfile.getId());