From a25e328b7c6c7bf503f0b137f13e600895abe729 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Sat, 11 Feb 2023 00:20:00 +0200 Subject: [PATCH 1/4] fixed update settings method --- .../server/controller/UserController.java | 31 ++++++---- .../controller/BaseUserControllerTest.java | 31 +++++----- .../server/dao/user/UserSettingsService.java | 5 +- .../dao/user/UserSettingsServiceImpl.java | 58 +++++++++++++------ 4 files changed, 80 insertions(+), 45 deletions(-) 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 ed55bcf324..a38ef27d62 100644 --- a/application/src/main/java/org/thingsboard/server/controller/UserController.java +++ b/application/src/main/java/org/thingsboard/server/controller/UserController.java @@ -92,7 +92,8 @@ import static org.thingsboard.server.controller.ControllerConstants.UUID_WIKI_LI public class UserController extends BaseController { public static final String USER_ID = "userId"; - public static final String JSON_PATHS = "jsonPaths"; + public static final String PATH = "path"; + public static final String PATHS = "paths"; public static final String YOU_DON_T_HAVE_PERMISSION_TO_PERFORM_THIS_OPERATION = "You don't have permission to perform this operation!"; public static final String ACTIVATE_URL_PATTERN = "%s/api/noauth/activate?activateToken=%s"; @@ -406,13 +407,21 @@ public class UserController extends BaseController { "{A:10, B:{C:10, D:5}}, E:6") @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") @PutMapping(value = "/user/settings") - public JsonNode putUserSettings(@RequestBody JsonNode settings) throws ThingsboardException { + public void putUserSettings(@RequestBody JsonNode settings) throws ThingsboardException { SecurityUser currentUser = getCurrentUser(); + userSettingsService.updateUserSettings(currentUser.getTenantId(), currentUser.getId(), settings); + } - UserSettings userSettings = new UserSettings(); - userSettings.setSettings(settings); - userSettings.setUserId(currentUser.getId()); - return userSettingsService.updateUserSettings(currentUser.getTenantId(), userSettings).getSettings(); + @ApiOperation(value = "Update user settings (saveUserSettings)", + notes = "Update user settings for authorized user. Only specified json elements will be updated." + + "Example: you have such settings: {A:5, B:{C:10, D:5}}. Updating it with {A:10, E:6} will result in" + + "{A:10, B:{C:10, D:5}}, E:6") + @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") + @PutMapping(value = "/user/settings/{path}") + public void putUserSettings(@ApiParam(value = PATH) + @PathVariable(PATH) String path, @RequestBody JsonNode settings) throws ThingsboardException { + SecurityUser currentUser = getCurrentUser(); + userSettingsService.updateUserSettings(currentUser.getTenantId(), currentUser.getId(), path, settings); } @ApiOperation(value = "Get user settings (getUserSettings)", @@ -430,12 +439,12 @@ public class UserController extends BaseController { notes = "Delete user settings by specifying list of json element xpaths. \n " + "Example: to delete B and C element in { \"A\": {\"B\": 5}, \"C\": 15} send A.B,C in jsonPaths request parameter" ) @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") - @RequestMapping(value = "/user/settings/{jsonPaths}", method = RequestMethod.DELETE) - public void deleteUserSettings( @ApiParam(value = JSON_PATHS) - @PathVariable(JSON_PATHS) String jsonPaths) throws ThingsboardException { - checkParameter(USER_ID, jsonPaths); + @RequestMapping(value = "/user/settings/{paths}", method = RequestMethod.DELETE) + public void deleteUserSettings(@ApiParam(value = PATHS) + @PathVariable(PATHS) String paths) throws ThingsboardException { + checkParameter(USER_ID, paths); SecurityUser currentUser = getCurrentUser(); - userSettingsService.deleteUserSettings(currentUser.getTenantId(), currentUser.getId(), Arrays.asList(jsonPaths.split(","))); + userSettingsService.deleteUserSettings(currentUser.getTenantId(), currentUser.getId(), Arrays.asList(paths.split(","))); } } 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 81111d2de4..4679ee4411 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java @@ -759,30 +759,33 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { public void testUpdateUserSettings() throws Exception { loginCustomerUser(); - JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":10, \"E\":18}"); + JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\":5, \"D\":5}}"); JsonNode savedSettings = doPost("/api/user/settings", userSettings, JsonNode.class); Assert.assertEquals(userSettings, savedSettings); - JsonNode newSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": 16}}"); - JsonNode updatedSettings = doPut("/api/user/settings", newSettings, JsonNode.class); - JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": 16}, \"E\":18}"); + JsonNode newSettings = mapper.readTree("{\"A\":10}"); + doPut("/api/user/settings", newSettings); + JsonNode updatedSettings = doGet("/api/user/settings", JsonNode.class); + JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":5, \"D\":5}}"); Assert.assertEquals(expectedSettings, updatedSettings); - JsonNode patchedSettings = mapper.readTree("\"C\":{\"E\": 22}}"); - updatedSettings = doPut("/api/user/settings", patchedSettings, JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"E\": 22}, \"E\":18}"); + JsonNode patchedSettings = mapper.readTree("{\"B\":{\"E\": 22}}"); + doPut("/api/user/settings", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": 22}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("\"C.D\": 16}}"); - updatedSettings = doPut("/api/user/settings", patchedSettings, JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": 16, \"E\": 22}, \"E\":18}"); + patchedSettings = mapper.readTree("{\"I\": 56}"); + doPut("/api/user/settings/B.E", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": {\"I\":56}}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("\"C.D\": {\"A\":5}}}"); - updatedSettings = doPut("/api/user/settings", patchedSettings, JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": {\"A\":5}, \"E\": 22}, \"E\":18}"); + patchedSettings = mapper.readTree("{\"I\": 76, \"F\": 92}"); + doPut("/api/user/settings/B.E", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": {\"I\":76, \"F\": 92}}}"); Assert.assertEquals(expectedSettings, updatedSettings); - } @Test diff --git a/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java b/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java index 30f9b9334a..0b2c6f5aef 100644 --- a/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java +++ b/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java @@ -15,6 +15,8 @@ */ package org.thingsboard.server.dao.user; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.security.UserSettings; @@ -23,7 +25,8 @@ import java.util.List; public interface UserSettingsService { - UserSettings updateUserSettings(TenantId tenantId, UserSettings userSettings); + void updateUserSettings(TenantId tenantId, UserId userId, JsonNode settings); + void updateUserSettings(TenantId tenantId, UserId userId, String path, JsonNode settings); UserSettings saveUserSettings(TenantId tenantId, UserSettings userSettings); diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java index dc1e18352c..0c41589b95 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java @@ -15,6 +15,8 @@ */ package org.thingsboard.server.dao.user; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -33,6 +35,7 @@ import org.thingsboard.server.dao.entity.AbstractCachedService; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Map; import static org.thingsboard.server.dao.service.Validator.validateId; @@ -51,13 +54,37 @@ public class UserSettingsServiceImpl extends AbstractCachedService>(){})); + try { + newUserSettings.setSettings(new ObjectMapper().readValue(dcSettings.jsonString(), ObjectNode.class)); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + doSaveUserSettings(tenantId, newUserSettings); } @Override @@ -78,11 +105,11 @@ public class UserSettingsServiceImpl extends AbstractCachedService fieldNames = updateNode.fieldNames(); while (fieldNames.hasNext()) { String fieldName = fieldNames.next(); - JsonNode jsonNode = mainNode.get(fieldName); - if (jsonNode != null && jsonNode.isObject()) { - merge(jsonNode, updateNode.get(fieldName)); - } else { - if (mainNode instanceof ObjectNode) { - JsonNode value = updateNode.get(fieldName); - ((ObjectNode) mainNode).set(fieldName, value); - } - } + JsonNode value = updateNode.get(fieldName); + ((ObjectNode) mainNode).set(fieldName, value); } return mainNode; } From 683dee03da78773e570959e7dfe1222be73b2b3a Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Sun, 12 Feb 2023 20:52:18 +0200 Subject: [PATCH 2/4] refactoring --- .../server/controller/UserController.java | 19 +---- .../controller/BaseUserControllerTest.java | 45 ++++++++--- .../server/dao/user/UserSettingsService.java | 2 - .../dao/user/UserSettingsServiceImpl.java | 75 +++++++++++++------ 4 files changed, 87 insertions(+), 54 deletions(-) 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 a38ef27d62..3e202de938 100644 --- a/application/src/main/java/org/thingsboard/server/controller/UserController.java +++ b/application/src/main/java/org/thingsboard/server/controller/UserController.java @@ -92,7 +92,6 @@ import static org.thingsboard.server.controller.ControllerConstants.UUID_WIKI_LI public class UserController extends BaseController { public static final String USER_ID = "userId"; - public static final String PATH = "path"; public static final String PATHS = "paths"; public static final String YOU_DON_T_HAVE_PERMISSION_TO_PERFORM_THIS_OPERATION = "You don't have permission to perform this operation!"; public static final String ACTIVATE_URL_PATTERN = "%s/api/noauth/activate?activateToken=%s"; @@ -403,8 +402,8 @@ public class UserController extends BaseController { @ApiOperation(value = "Update user settings (saveUserSettings)", notes = "Update user settings for authorized user. Only specified json elements will be updated." + - "Example: you have such settings: {A:5, B:{C:10, D:5}}. Updating it with {A:10, E:6} will result in" + - "{A:10, B:{C:10, D:5}}, E:6") + "Example: you have such settings: {A:5, B:{C:10, D:20}}. Updating it with {B:{C:10, D:30}} will result in" + + "{A:5, B:{C:10, D:30}}. The same could be achieved by putting {B.D:30}") @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") @PutMapping(value = "/user/settings") public void putUserSettings(@RequestBody JsonNode settings) throws ThingsboardException { @@ -412,20 +411,8 @@ public class UserController extends BaseController { userSettingsService.updateUserSettings(currentUser.getTenantId(), currentUser.getId(), settings); } - @ApiOperation(value = "Update user settings (saveUserSettings)", - notes = "Update user settings for authorized user. Only specified json elements will be updated." + - "Example: you have such settings: {A:5, B:{C:10, D:5}}. Updating it with {A:10, E:6} will result in" + - "{A:10, B:{C:10, D:5}}, E:6") - @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") - @PutMapping(value = "/user/settings/{path}") - public void putUserSettings(@ApiParam(value = PATH) - @PathVariable(PATH) String path, @RequestBody JsonNode settings) throws ThingsboardException { - SecurityUser currentUser = getCurrentUser(); - userSettingsService.updateUserSettings(currentUser.getTenantId(), currentUser.getId(), path, settings); - } - @ApiOperation(value = "Get user settings (getUserSettings)", - notes = "Fetch the User settings based on the provided User Id. " ) + notes = "Fetch the User settings based on authorized user. " ) @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") @GetMapping(value = "/user/settings") public JsonNode getUserSettings() throws ThingsboardException { 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 4679ee4411..d15841f5a2 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java @@ -29,7 +29,6 @@ import org.springframework.context.annotation.Primary; import org.springframework.http.HttpHeaders; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.ResultActions; -import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.common.data.Customer; import org.thingsboard.server.common.data.StringUtils; import org.thingsboard.server.common.data.Tenant; @@ -41,7 +40,6 @@ 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.security.UserSettings; import org.thingsboard.server.dao.exception.DataValidationException; import org.thingsboard.server.dao.user.UserDao; import org.thingsboard.server.service.mail.TestMailService; @@ -755,39 +753,62 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { Assert.assertEquals(retrievedSettings, userSettings); } + @Test + public void testShouldNotSaveJsonWithRestrictedSymbols() throws Exception { + loginCustomerUser(); + + JsonNode userSettings = mapper.readTree("{\"A.B\":5, \"E\":18}"); + doPost("/api/user/settings", userSettings).andExpect(status().isBadRequest()); + + userSettings = mapper.readTree("{\"A,B\":5, \"E\":18}"); + doPost("/api/user/settings", userSettings).andExpect(status().isBadRequest()); + } + @Test public void testUpdateUserSettings() throws Exception { loginCustomerUser(); - JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\":5, \"D\":5}}"); + JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); JsonNode savedSettings = doPost("/api/user/settings", userSettings, JsonNode.class); Assert.assertEquals(userSettings, savedSettings); JsonNode newSettings = mapper.readTree("{\"A\":10}"); doPut("/api/user/settings", newSettings); JsonNode updatedSettings = doGet("/api/user/settings", JsonNode.class); - JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":5, \"D\":5}}"); + JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - JsonNode patchedSettings = mapper.readTree("{\"B\":{\"E\": 22}}"); + JsonNode patchedSettings = mapper.readTree("{\"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": 22}}"); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("{\"I\": 56}"); - doPut("/api/user/settings/B.E", patchedSettings); + patchedSettings = mapper.readTree("{\"B.D\": {\"E\": 56}}"); + doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": {\"I\":56}}}"); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\": 56}}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("{\"I\": 76, \"F\": 92}"); - doPut("/api/user/settings/B.E", patchedSettings); + patchedSettings = mapper.readTree("{\"B.D\": {\"E\": 76, \"F\": 92}}"); + doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": {\"I\":76, \"F\": 92}}}"); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\":76, \"F\": 92}}}"); Assert.assertEquals(expectedSettings, updatedSettings); } + @Test + public void testShouldNotUpdateUserSettingsWithNoExistingPath() throws Exception { + loginCustomerUser(); + + JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); + JsonNode savedSettings = doPost("/api/user/settings", userSettings, JsonNode.class); + Assert.assertEquals(userSettings, savedSettings); + + JsonNode newSettings = mapper.readTree("{\"A.E\":10}"); + doPut("/api/user/settings", newSettings).andExpect(status().isBadRequest()); + } + @Test public void testDeleteUserSettings() throws Exception { loginCustomerUser(); diff --git a/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java b/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java index 0b2c6f5aef..2c6b149607 100644 --- a/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java +++ b/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java @@ -15,7 +15,6 @@ */ package org.thingsboard.server.dao.user; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; @@ -26,7 +25,6 @@ import java.util.List; public interface UserSettingsService { void updateUserSettings(TenantId tenantId, UserId userId, JsonNode settings); - void updateUserSettings(TenantId tenantId, UserId userId, String path, JsonNode settings); UserSettings saveUserSettings(TenantId tenantId, UserSettings userSettings); diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java index 0c41589b95..a87d53e49d 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java @@ -20,8 +20,10 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.github.fge.jackson.NodeType; import com.jayway.jsonpath.DocumentContext; import com.jayway.jsonpath.JsonPath; +import com.jayway.jsonpath.PathNotFoundException; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -31,6 +33,7 @@ import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.security.UserSettings; import org.thingsboard.server.dao.entity.AbstractCachedService; +import org.thingsboard.server.dao.exception.DataValidationException; import java.util.ArrayList; import java.util.Iterator; @@ -67,26 +70,6 @@ public class UserSettingsServiceImpl extends AbstractCachedService>(){})); - try { - newUserSettings.setSettings(new ObjectMapper().readValue(dcSettings.jsonString(), ObjectNode.class)); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } - doSaveUserSettings(tenantId, newUserSettings); - } - @Override public UserSettings findUserSettings(TenantId tenantId, UserId userId) { log.trace("Executing findUserSettings for user [{}]", userId); @@ -119,7 +102,7 @@ public class UserSettingsServiceImpl extends AbstractCachedService fieldNames = userSettings.fieldNames(); + while (fieldNames.hasNext()) { + String fieldName = fieldNames.next(); + if (fieldName.contains(".") || fieldName.contains(",")) { + throw new DataValidationException("Json field name should not contain \".\" or \",\" symbols"); + } + } + } + public JsonNode update(JsonNode mainNode, JsonNode updateNode) { + DocumentContext dcOldSettings = JsonPath.parse(mainNode.toString()); Iterator fieldNames = updateNode.fieldNames(); while (fieldNames.hasNext()) { String fieldName = fieldNames.next(); - JsonNode value = updateNode.get(fieldName); - ((ObjectNode) mainNode).set(fieldName, value); + validatePathExists(dcOldSettings, fieldName); + dcOldSettings = dcOldSettings.set("$." + fieldName, getValueByNodeType(updateNode.get(fieldName))); + } + try { + return new ObjectMapper().readValue(dcOldSettings.jsonString(), ObjectNode.class); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + private static void validatePathExists(DocumentContext dcOldSettings, String fieldName) { + try { + dcOldSettings.read("$." + fieldName); + }catch (PathNotFoundException e) { + throw new DataValidationException("Json element with path " + fieldName + "was not found"); + } + } + + private static Object getValueByNodeType(final JsonNode value) + { + final NodeType type = NodeType.getNodeType(value); + switch (type) { + case STRING: + return value.textValue(); + case NUMBER: + case INTEGER: + return value.bigIntegerValue(); + case NULL: + case ARRAY: + return value; + case OBJECT: + return new ObjectMapper().convertValue(value, new TypeReference>() {}); + case BOOLEAN: + return value.booleanValue(); + default: + throw new UnsupportedOperationException(); } - return mainNode; } } From 4a9c4cc6148170ec9ee7eb7de10ac836d60b43f2 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Sun, 12 Feb 2023 20:59:52 +0200 Subject: [PATCH 3/4] updated test --- .../server/controller/BaseUserControllerTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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 d15841f5a2..f7c41b0fc6 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java @@ -784,10 +784,10 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("{\"B.D\": {\"E\": 56}}"); + patchedSettings = mapper.readTree("{\"B.D\": \"stringValue3\"}"); doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\": 56}}}"); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": \"stringValue3\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); patchedSettings = mapper.readTree("{\"B.D\": {\"E\": 76, \"F\": 92}}"); @@ -795,6 +795,12 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { updatedSettings = doGet("/api/user/settings", JsonNode.class); expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\":76, \"F\": 92}}}"); Assert.assertEquals(expectedSettings, updatedSettings); + + patchedSettings = mapper.readTree("{\"B.D.E\": 100}"); + doPut("/api/user/settings", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\":100, \"F\": 92}}}"); + Assert.assertEquals(expectedSettings, updatedSettings); } @Test From f812e02920e02d2e6b02e508ac1ba5e629c29463 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Sun, 12 Feb 2023 23:40:12 +0200 Subject: [PATCH 4/4] added possibility to specify non existing elements --- .../controller/BaseUserControllerTest.java | 45 +++++++++++++++---- .../dao/user/UserSettingsServiceImpl.java | 15 ++++--- 2 files changed, 46 insertions(+), 14 deletions(-) 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 f7c41b0fc6..1e49974b28 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java @@ -778,41 +778,68 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - JsonNode patchedSettings = mapper.readTree("{\"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); + JsonNode patchedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); patchedSettings = mapper.readTree("{\"B.D\": \"stringValue3\"}"); doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": \"stringValue3\"}}"); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\": \"stringValue3\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); patchedSettings = mapper.readTree("{\"B.D\": {\"E\": 76, \"F\": 92}}"); doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\":76, \"F\": 92}}}"); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\": {\"E\":76, \"F\": 92}}}"); Assert.assertEquals(expectedSettings, updatedSettings); patchedSettings = mapper.readTree("{\"B.D.E\": 100}"); doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\":100, \"F\": 92}}}"); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\": {\"E\":100, \"F\": 92}}}"); Assert.assertEquals(expectedSettings, updatedSettings); } @Test - public void testShouldNotUpdateUserSettingsWithNoExistingPath() throws Exception { + public void testShouldCreatePathIfNotExists() throws Exception { loginCustomerUser(); - JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); + JsonNode userSettings = mapper.readTree("{\"A\":5}"); JsonNode savedSettings = doPost("/api/user/settings", userSettings, JsonNode.class); Assert.assertEquals(userSettings, savedSettings); - JsonNode newSettings = mapper.readTree("{\"A.E\":10}"); - doPut("/api/user/settings", newSettings).andExpect(status().isBadRequest()); + JsonNode newSettings = mapper.readTree("{\"B\":{\"C\": 10}}"); + doPut("/api/user/settings", newSettings); + JsonNode updatedSettings = doGet("/api/user/settings", JsonNode.class); + JsonNode expectedSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\": 10}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"B.K\":true}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\": 10, \"K\": true}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"B\":{}}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"F.G\":\"string\"}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{}, \"F\":{\"G\": \"string\"}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"F\":{\"G\":\"string2\"}}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{}, \"F\":{\"G\": \"string2\"}}"); + Assert.assertEquals(expectedSettings, updatedSettings); } @Test diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java index a87d53e49d..593a8846cc 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java @@ -37,6 +37,7 @@ import org.thingsboard.server.dao.exception.DataValidationException; import java.util.ArrayList; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -135,7 +136,7 @@ public class UserSettingsServiceImpl extends AbstractCachedService fieldNames = updateNode.fieldNames(); while (fieldNames.hasNext()) { String fieldName = fieldNames.next(); - validatePathExists(dcOldSettings, fieldName); + createPathIfNotExists(dcOldSettings, "$."+ fieldName); dcOldSettings = dcOldSettings.set("$." + fieldName, getValueByNodeType(updateNode.get(fieldName))); } try { @@ -145,11 +146,15 @@ public class UserSettingsServiceImpl extends AbstractCachedService()); + return path; } }