diff --git a/application/src/main/java/org/thingsboard/server/controller/UserController.java b/application/src/main/java/org/thingsboard/server/controller/UserController.java index 341ec387e9..4536d77a62 100644 --- a/application/src/main/java/org/thingsboard/server/controller/UserController.java +++ b/application/src/main/java/org/thingsboard/server/controller/UserController.java @@ -504,7 +504,7 @@ public class UserController extends BaseController { @ApiOperation(value = "Report action of User over the dashboard (reportUserDashboardAction)", notes = "Enables or Disables user credentials. Useful when you would like to block user account without deleting it. " + PAGE_DATA_PARAMETERS + TENANT_AUTHORITY_PARAGRAPH) @PreAuthorize("hasAnyAuthority('TENANT_ADMIN', 'CUSTOMER_USER')") - @RequestMapping(value = "/user/dashboards/{dashboardId}/{action}", method = RequestMethod.POST) + @RequestMapping(value = "/user/dashboards/{dashboardId}/{action}", method = RequestMethod.GET) @ResponseBody public UserDashboardsInfo reportUserDashboardAction( @ApiParam(value = DASHBOARD_ID_PARAM_DESCRIPTION) diff --git a/application/src/main/java/org/thingsboard/server/service/entitiy/user/DefaultTbUserSettingsService.java b/application/src/main/java/org/thingsboard/server/service/entitiy/user/DefaultTbUserSettingsService.java index add2343e84..20a20a4bfc 100644 --- a/application/src/main/java/org/thingsboard/server/service/entitiy/user/DefaultTbUserSettingsService.java +++ b/application/src/main/java/org/thingsboard/server/service/entitiy/user/DefaultTbUserSettingsService.java @@ -18,6 +18,7 @@ package org.thingsboard.server.service.entitiy.user; import com.fasterxml.jackson.databind.JsonNode; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; import org.springframework.stereotype.Service; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.common.data.HasTitle; @@ -35,8 +36,11 @@ import org.thingsboard.server.dao.dashboard.DashboardService; import org.thingsboard.server.dao.user.UserSettingsService; import org.thingsboard.server.queue.util.TbCoreComponent; +import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.function.Function; import java.util.function.Predicate; @@ -49,7 +53,7 @@ import java.util.stream.Collectors; public class DefaultTbUserSettingsService implements TbUserSettingsService { private static final int MAX_DASHBOARD_INFO_LIST_SIZE = 10; - private static final Predicate EMPTY_TITLE = i -> StringUtils.isNotEmpty(i.getTitle()); + private static final Predicate EMPTY_TITLE = i -> StringUtils.isEmpty(i.getTitle()); private final UserSettingsService settingsService; private final DashboardService dashboardService; @@ -96,24 +100,7 @@ public class DefaultTbUserSettingsService implements TbUserSettingsService { return UserDashboardsInfo.EMPTY; } UserDashboardsInfo stored = JacksonUtil.convertValue(us.getSettings(), UserDashboardsInfo.class); - if (stored == null) { - return UserDashboardsInfo.EMPTY; - } - - if (!stored.getLast().isEmpty()) { - stored.getLast().forEach(i -> setTitleIfEmpty(tenantId, i)); - stored.getLast().removeIf(EMPTY_TITLE); - } - if (!stored.getStarred().isEmpty()) { - Map lastMap = stored.getLast().stream().collect(Collectors.toMap(LastVisitedDashboardInfo::getId, Function.identity())); - stored.getStarred().forEach(i -> { - var last = lastMap.get(i.getId()); - i.setTitle(last != null ? last.getTitle() : null); - }); - stored.getStarred().forEach(i -> setTitleIfEmpty(tenantId, i)); - stored.getStarred().removeIf(EMPTY_TITLE); - } - return stored; + return getUserDashboardsInfo(tenantId, stored); } @Override @@ -144,12 +131,26 @@ public class DefaultTbUserSettingsService implements TbUserSettingsService { us.setType(UserSettings.STARRED_DASHBOARDS); us.setSettings(JacksonUtil.valueToTree(stored)); saveUserSettings(tenantId, us); - return stored; + return getUserDashboardsInfo(tenantId, stored); } private void addToVisited(UserDashboardsInfo stored, DashboardId dashboardId) { UUID id = dashboardId.getId(); - stored.getStarred().removeIf(d -> id.equals(d.getId())); + long ts = System.currentTimeMillis(); + var opt = stored.getLast().stream().filter(d -> id.equals(d.getId())).findFirst(); + if (opt.isPresent()) { + opt.get().setLastVisited(ts); + } else { + var newInfo = new LastVisitedDashboardInfo(); + newInfo.setId(id); + newInfo.setStarred(stored.getStarred().stream().anyMatch(d -> id.equals(d.getId()))); + newInfo.setLastVisited(System.currentTimeMillis()); + stored.getLast().add(newInfo); + } + stored.getLast().sort(Comparator.comparing(LastVisitedDashboardInfo::getLastVisited).reversed()); + if (stored.getLast().size() > MAX_DASHBOARD_INFO_LIST_SIZE) { + stored.setLast(stored.getLast().stream().limit(MAX_DASHBOARD_INFO_LIST_SIZE).collect(Collectors.toList())); + } } private void removeFromStarred(UserDashboardsInfo stored, DashboardId dashboardId) { @@ -170,9 +171,13 @@ public class DefaultTbUserSettingsService implements TbUserSettingsService { newInfo.setStarredAt(System.currentTimeMillis()); stored.getStarred().add(newInfo); } - stored.getLast().stream().filter(d -> id.equals(d.getId())).forEach(d -> d.setStarred(true)); - //TODO: self-heal if some of the dashboards were deleted. - //TODO: limit by size. + stored.getStarred().sort(Comparator.comparing(StarredDashboardInfo::getStarredAt).reversed()); + if (stored.getStarred().size() > MAX_DASHBOARD_INFO_LIST_SIZE) { + stored.setStarred(stored.getStarred().stream().limit(MAX_DASHBOARD_INFO_LIST_SIZE).collect(Collectors.toList())); + } + Set starredMap = + stored.getStarred().stream().map(AbstractUserDashboardInfo::getId).collect(Collectors.toSet()); + stored.getLast().forEach(d -> d.setStarred(starredMap.contains(d.getId()))); } private void setTitleIfEmpty(TenantId tenantId, AbstractUserDashboardInfo i) { @@ -182,5 +187,25 @@ public class DefaultTbUserSettingsService implements TbUserSettingsService { } } + private UserDashboardsInfo getUserDashboardsInfo(TenantId tenantId, UserDashboardsInfo stored) { + if (stored == null) { + return UserDashboardsInfo.EMPTY; + } + + if (!stored.getLast().isEmpty()) { + stored.getLast().forEach(i -> setTitleIfEmpty(tenantId, i)); + stored.getLast().removeIf(EMPTY_TITLE); + } + if (!stored.getStarred().isEmpty()) { + Map lastMap = stored.getLast().stream().collect(Collectors.toMap(LastVisitedDashboardInfo::getId, Function.identity())); + stored.getStarred().forEach(i -> { + var last = lastMap.get(i.getId()); + i.setTitle(last != null ? last.getTitle() : null); + }); + stored.getStarred().forEach(i -> setTitleIfEmpty(tenantId, i)); + stored.getStarred().removeIf(EMPTY_TITLE); + } + return stored; + } } diff --git a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java index 7149f52630..84fab2d971 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java @@ -31,6 +31,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.ResultActions; import org.thingsboard.server.common.data.Customer; +import org.thingsboard.server.common.data.Dashboard; import org.thingsboard.server.common.data.StringUtils; import org.thingsboard.server.common.data.Tenant; import org.thingsboard.server.common.data.User; @@ -42,6 +43,8 @@ import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.Authority; +import org.thingsboard.server.common.data.settings.StarredDashboardInfo; +import org.thingsboard.server.common.data.settings.UserDashboardsInfo; import org.thingsboard.server.dao.exception.DataValidationException; import org.thingsboard.server.dao.user.UserDao; @@ -356,7 +359,7 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { String userIdStr = savedUser.getId().getId().toString(); doGet("/api/user/" + userIdStr) .andExpect(status().isNotFound()) - .andExpect(statusReason(containsString( msgErrorNoFound("User",userIdStr)))); + .andExpect(statusReason(containsString(msgErrorNoFound("User", userIdStr)))); } @Test @@ -574,9 +577,9 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { String email1 = "testEmail1"; String email2 = "testEmail2"; List customerUsersEmail1 = new ArrayList<>(); - List customerUsersEmail2= new ArrayList<>(); + List customerUsersEmail2 = new ArrayList<>(); for (int i = 0; i < 45; i++) { - User customerUser = createCustomerUser( customerId); + User customerUser = createCustomerUser(customerId); customerUser.setEmail(email1 + StringUtils.randomAlphanumeric((int) (5 + Math.random() * 10)) + "@thingsboard.org"); customerUsersEmail1.add(doPost("/api/user", customerUser, User.class)); @@ -685,7 +688,7 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { JsonNode retrievedSettings = doGet("/api/user/settings", JsonNode.class); Assert.assertEquals(retrievedSettings, userSettings); - } + } @Test public void testShouldNotSaveJsonWithRestrictedSymbols() throws Exception { @@ -860,7 +863,7 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { List expectedUserInfos = customerUsersContainingWord.stream().map(customerUser -> new UserEmailInfo(customerUser.getId(), customerUser.getEmail(), customerUser.getFirstName() == null ? "" : customerUser.getFirstName(), - customerUser.getLastName() == null ? "" : customerUser.getLastName())) + customerUser.getLastName() == null ? "" : customerUser.getLastName())) .sorted(userDataIdComparator).collect(Collectors.toList()); usersInfo.sort(userDataIdComparator); @@ -912,8 +915,8 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { List usersInfo = getUsersInfo(pageLink); List expectedUserInfos = usersContainingWord.stream().map(customerUser -> new UserEmailInfo(customerUser.getId(), - customerUser.getEmail(), customerUser.getFirstName() == null ? "" : customerUser.getFirstName(), - customerUser.getLastName() == null ? "" : customerUser.getLastName())) + customerUser.getEmail(), customerUser.getFirstName() == null ? "" : customerUser.getFirstName(), + customerUser.getLastName() == null ? "" : customerUser.getLastName())) .sorted(userDataIdComparator).collect(Collectors.toList()); usersInfo.sort(userDataIdComparator); @@ -922,7 +925,7 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { // find user by full last name pageLink = new PageLink(10, 0, searchText + "3"); usersInfo = getUsersInfo(pageLink); - Assert.assertEquals(2, usersInfo.size()); + Assert.assertEquals(2, usersInfo.size()); //clear users doDelete("/api/customer/" + customerId.getId().toString()) @@ -941,6 +944,7 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { private static User createCustomerUser(CustomerId customerId) { return createCustomerUser(null, null, customerId); } + private static User createCustomerUser(String firstName, String lastName, CustomerId customerId) { String suffix = StringUtils.randomAlphanumeric((int) (5 + Math.random() * 10)); return createCustomerUser(firstName, lastName, "testMail" + suffix + "@thingsboard.org", customerId); @@ -959,6 +963,7 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { private User createTenantAdminUser() { return createTenantAdminUser(null, null); } + private User createTenantAdminUser(String firstName, String lastName) { String suffix = StringUtils.randomAlphanumeric((int) (5 + Math.random() * 10)); @@ -975,7 +980,8 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { List loadedCustomerUsers = new ArrayList<>(); PageData pageData = null; do { - pageData = doGetTypedWithPageLink("/api/users/info?", new TypeReference<>() {}, pageLink); + pageData = doGetTypedWithPageLink("/api/users/info?", new TypeReference<>() { + }, pageLink); loadedCustomerUsers.addAll(pageData.getData()); if (pageData.hasNext()) { pageLink = pageLink.nextPageLink(); @@ -984,4 +990,119 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { return loadedCustomerUsers; } + @Test + public void testEmptyDashboardSettings() throws Exception { + loginCustomerUser(); + + UserDashboardsInfo retrievedSettings = doGet("/api/user/dashboards", UserDashboardsInfo.class); + Assert.assertNotNull(retrievedSettings); + Assert.assertNotNull(retrievedSettings.getLast()); + Assert.assertTrue(retrievedSettings.getLast().isEmpty()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertTrue(retrievedSettings.getStarred().isEmpty()); + } + + @Test + public void testDashboardSettingsFlow() throws Exception { + loginTenantAdmin(); + + Dashboard dashboard1 = new Dashboard(); + dashboard1.setTitle("My dashboard 1"); + Dashboard savedDashboard1 = doPost("/api/dashboard", dashboard1, Dashboard.class); + Dashboard dashboard2 = new Dashboard(); + dashboard2.setTitle("My dashboard 2"); + Dashboard savedDashboard2 = doPost("/api/dashboard", dashboard2, Dashboard.class); + + UserDashboardsInfo retrievedSettings = doGet("/api/user/dashboards", UserDashboardsInfo.class); + Assert.assertNotNull(retrievedSettings); + Assert.assertNotNull(retrievedSettings.getLast()); + Assert.assertTrue(retrievedSettings.getLast().isEmpty()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertTrue(retrievedSettings.getStarred().isEmpty()); + + UserDashboardsInfo newSettings = doGet("/api/user/dashboards/" + savedDashboard1.getId().getId() + "/visit", UserDashboardsInfo.class); + Assert.assertNotNull(newSettings); + Assert.assertNotNull(newSettings.getLast()); + Assert.assertEquals(1, newSettings.getLast().size()); + var lastVisited = newSettings.getLast().get(0); + Assert.assertEquals(savedDashboard1.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard1.getTitle(), lastVisited.getTitle()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertTrue(retrievedSettings.getStarred().isEmpty()); + + newSettings = doGet("/api/user/dashboards/" + savedDashboard2.getId().getId() + "/visit", UserDashboardsInfo.class); + Assert.assertNotNull(newSettings); + Assert.assertNotNull(newSettings.getLast()); + Assert.assertEquals(2, newSettings.getLast().size()); + lastVisited = newSettings.getLast().get(0); + Assert.assertEquals(savedDashboard2.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard2.getTitle(), lastVisited.getTitle()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertTrue(retrievedSettings.getStarred().isEmpty()); + + newSettings = doGet("/api/user/dashboards", UserDashboardsInfo.class); + Assert.assertNotNull(newSettings); + Assert.assertNotNull(newSettings.getLast()); + Assert.assertEquals(2, newSettings.getLast().size()); + lastVisited = newSettings.getLast().get(0); + Assert.assertEquals(savedDashboard2.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard2.getTitle(), lastVisited.getTitle()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertTrue(retrievedSettings.getStarred().isEmpty()); + + newSettings = doGet("/api/user/dashboards/" + savedDashboard1.getId().getId() + "/star", UserDashboardsInfo.class); + Assert.assertNotNull(newSettings); + Assert.assertNotNull(newSettings.getLast()); + Assert.assertEquals(2, newSettings.getLast().size()); + lastVisited = newSettings.getLast().get(0); + Assert.assertEquals(savedDashboard2.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard2.getTitle(), lastVisited.getTitle()); + Assert.assertFalse(lastVisited.isStarred()); + lastVisited = newSettings.getLast().get(1); + Assert.assertEquals(savedDashboard1.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard1.getTitle(), lastVisited.getTitle()); + Assert.assertTrue(lastVisited.isStarred()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertEquals(1, newSettings.getStarred().size()); + StarredDashboardInfo starred = newSettings.getStarred().get(0); + Assert.assertEquals(savedDashboard1.getId().getId(), starred.getId()); + Assert.assertEquals(savedDashboard1.getTitle(), starred.getTitle()); + + newSettings = doGet("/api/user/dashboards/" + savedDashboard2.getId().getId() + "/star", UserDashboardsInfo.class); + Assert.assertNotNull(newSettings); + Assert.assertNotNull(newSettings.getLast()); + Assert.assertEquals(2, newSettings.getLast().size()); + lastVisited = newSettings.getLast().get(0); + Assert.assertEquals(savedDashboard2.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard2.getTitle(), lastVisited.getTitle()); + Assert.assertTrue(lastVisited.isStarred()); + lastVisited = newSettings.getLast().get(1); + Assert.assertEquals(savedDashboard1.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard1.getTitle(), lastVisited.getTitle()); + Assert.assertTrue(lastVisited.isStarred()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertEquals(2, newSettings.getStarred().size()); + starred = newSettings.getStarred().get(0); + Assert.assertEquals(savedDashboard2.getId().getId(), starred.getId()); + Assert.assertEquals(savedDashboard2.getTitle(), starred.getTitle()); + + newSettings = doGet("/api/user/dashboards/" + savedDashboard1.getId().getId() + "/unstar", UserDashboardsInfo.class); + Assert.assertNotNull(newSettings); + Assert.assertNotNull(newSettings.getLast()); + Assert.assertEquals(2, newSettings.getLast().size()); + lastVisited = newSettings.getLast().get(0); + Assert.assertEquals(savedDashboard2.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard2.getTitle(), lastVisited.getTitle()); + Assert.assertTrue(lastVisited.isStarred()); + lastVisited = newSettings.getLast().get(1); + Assert.assertEquals(savedDashboard1.getId().getId(), lastVisited.getId()); + Assert.assertEquals(savedDashboard1.getTitle(), lastVisited.getTitle()); + Assert.assertFalse(lastVisited.isStarred()); + Assert.assertNotNull(retrievedSettings.getStarred()); + Assert.assertEquals(1, newSettings.getStarred().size()); + starred = newSettings.getStarred().get(0); + Assert.assertEquals(savedDashboard2.getId().getId(), starred.getId()); + Assert.assertEquals(savedDashboard2.getTitle(), starred.getTitle()); + } + } diff --git a/dao/src/main/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDao.java b/dao/src/main/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDao.java index e796faefdb..c0b270ccbb 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDao.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDao.java @@ -19,7 +19,6 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.thingsboard.server.common.data.id.TenantId; -import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.settings.UserSettings; import org.thingsboard.server.common.data.settings.UserSettingsCompositeKey; import org.thingsboard.server.dao.DaoUtil; @@ -42,13 +41,13 @@ public class JpaUserSettingsDao extends JpaAbstractDaoListeningExecutorService i } @Override - public UserSettings findById(TenantId tenantId, UserId userId, String type) { - return DaoUtil.getData(userSettingsRepository.findById(new UserSettingsCompositeKey(userId.getId(), type))); + public UserSettings findById(TenantId tenantId, UserSettingsCompositeKey id) { + return DaoUtil.getData(userSettingsRepository.findById(id)); } @Override - public void removeById(TenantId tenantId, UserId userId, String type) { - userSettingsRepository.deleteById(new UserSettingsCompositeKey(userId.getId(), type)); + public void removeById(TenantId tenantId, UserSettingsCompositeKey id) { + userSettingsRepository.deleteById(id); } } diff --git a/dao/src/test/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDaoTest.java b/dao/src/test/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDaoTest.java index b5b578947d..d161af6b21 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDaoTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/sql/user/JpaUserSettingsDaoTest.java @@ -28,6 +28,7 @@ import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.security.Authority; import org.thingsboard.server.common.data.settings.UserSettings; +import org.thingsboard.server.common.data.settings.UserSettingsCompositeKey; import org.thingsboard.server.dao.AbstractJpaDaoTest; import org.thingsboard.server.dao.service.AbstractServiceTest; import org.thingsboard.server.dao.user.UserDao; @@ -66,12 +67,12 @@ public class JpaUserSettingsDaoTest extends AbstractJpaDaoTest { public void testFindSettingsByUserId() { UserSettings userSettings = createUserSettings(user.getId()); - UserSettings retrievedUserSettings = userSettingsDao.findById(SYSTEM_TENANT_ID, user.getId(), UserSettings.GENERAL); + UserSettings retrievedUserSettings = userSettingsDao.findById(SYSTEM_TENANT_ID, new UserSettingsCompositeKey(user.getId().getId(), UserSettings.GENERAL)); assertEquals(retrievedUserSettings.getSettings(), userSettings.getSettings()); - userSettingsDao.removeById(SYSTEM_TENANT_ID, user.getId(), UserSettings.GENERAL); + userSettingsDao.removeById(SYSTEM_TENANT_ID, new UserSettingsCompositeKey(user.getId().getId(), UserSettings.GENERAL)); - UserSettings retrievedUserSettings2 = userSettingsDao.findById(SYSTEM_TENANT_ID, user.getId(), UserSettings.GENERAL); + UserSettings retrievedUserSettings2 = userSettingsDao.findById(SYSTEM_TENANT_ID, new UserSettingsCompositeKey(user.getId().getId(), UserSettings.GENERAL)); assertNull(retrievedUserSettings2); }