logic change after review

application/src/main/java/org/thingsboard/server/service/rule/DefaultTbRuleChainService.java

@@ -19,7 +19,6 @@ import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.stereotype.Service;
 import org.thingsboard.common.util.JacksonUtil;
-import org.thingsboard.rule.engine.api.TbNodeException;
 import org.thingsboard.rule.engine.flow.TbRuleChainInputNode;
 import org.thingsboard.rule.engine.flow.TbRuleChainInputNodeConfiguration;
 import org.thingsboard.rule.engine.flow.TbRuleChainOutputNode;
@@ -405,21 +404,16 @@ public class DefaultTbRuleChainService extends AbstractTbEntityService implement
                 if (fromVersion < toVersion) {
                     log.debug("Going to upgrade rule node with id: {} type: {} fromVersion: {} toVersion: {}",
                             ruleNodeId, ruleNodeType, fromVersion, toVersion);
-                    try {
-                        TbNodeUpgradeUtils.upgradeConfigurationAndVersion(node, ruleNodeClass);
-                        log.debug("Successfully upgrade rule node with id: {} type: {}, rule chain id: {} fromVersion: {} toVersion: {}",
-                                ruleNodeId, ruleNodeType, ruleChainId, fromVersion, toVersion);
-                    } catch (TbNodeException e) {
-                        log.warn("Failed to upgrade rule node with id: {} type: {} rule chain id: {} fromVersion: {} toVersion: {} due to: ",
-                                ruleNodeId, ruleNodeType, ruleChainId, fromVersion, toVersion, e);
-                    }
+                    TbNodeUpgradeUtils.upgradeConfigurationAndVersion(node, ruleNodeClass);
+                    log.debug("Successfully upgrade rule node with id: {} type: {}, rule chain id: {} fromVersion: {} toVersion: {}",
+                            ruleNodeId, ruleNodeType, ruleChainId, fromVersion, toVersion);
                 } else {
                     log.debug("Rule node with id: {} type: {} ruleChainId: {} already set to latest version!",
                             ruleNodeId, ruleChainId, ruleNodeType);
                 }
             }
         } catch (Exception e) {
-            log.error("Failed to update the rule node with id: {} type: {}, rule chain id: {}",
+            log.error("Failed to upgrade rule node with id: {} type: {}, rule chain id: {}",
                     ruleNodeId, ruleNodeType, ruleChainId, e);
         }
         return node;

application/src/main/java/org/thingsboard/server/utils/TbNodeUpgradeUtils.java

@@ -16,6 +16,8 @@
 package org.thingsboard.server.utils;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
 import org.thingsboard.common.util.JacksonUtil;
 import org.thingsboard.rule.engine.api.NodeConfiguration;
 import org.thingsboard.rule.engine.api.TbNode;
@@ -24,37 +26,51 @@ import org.thingsboard.server.common.data.rule.RuleNode;
 import org.thingsboard.server.common.data.util.TbPair;
 import org.thingsboard.server.service.component.RuleNodeClassInfo;
 
+@Slf4j
 public class TbNodeUpgradeUtils {
 
-    public static void upgradeConfigurationAndVersion(RuleNode node, RuleNodeClassInfo nodeInfo) throws Exception {
+    public static void upgradeConfigurationAndVersion(RuleNode node, RuleNodeClassInfo nodeInfo) {
         JsonNode oldConfiguration = node.getConfiguration();
+        int configurationVersion = node.getConfigurationVersion();
+
+        int currentVersion = nodeInfo.getCurrentVersion();
         var configClass = nodeInfo.getAnnotation().configClazz();
 
         if (oldConfiguration == null || !oldConfiguration.isObject()) {
-            node.setConfiguration(JacksonUtil.valueToTree(configClass.getDeclaredConstructor().newInstance().defaultConfiguration()));
+            log.warn("Failed to upgrade rule node with id: {} type: {} fromVersion: {} toVersion: {}. " +
+                            "Current configuration is null or not a json object. " +
+                            "Going to set default configuration ... ",
+                    node.getId(), node.getType(), configurationVersion, currentVersion);
+            node.setConfiguration(getDefaultConfig(configClass));
         } else {
-            var tbVersionedNode = (TbNode) nodeInfo.getClazz().getDeclaredConstructor().newInstance();
+            var tbVersionedNode = getTbVersionedNode(nodeInfo);
             try {
-                TbPair<Boolean, JsonNode> upgradeResult = tbVersionedNode.upgrade(node.getConfigurationVersion(), oldConfiguration);
+                TbPair<Boolean, JsonNode> upgradeResult = tbVersionedNode.upgrade(configurationVersion, oldConfiguration);
                 if (upgradeResult.getFirst()) {
                     node.setConfiguration(upgradeResult.getSecond());
                 }
             } catch (TbNodeException e) {
-                if (!isValidConfig(oldConfiguration, configClass)) {
-                    throw e;
+                try {
+                    JacksonUtil.treeToValue(oldConfiguration, configClass);
+                } catch (Exception ex) {
+                    log.warn("Failed to upgrade rule node with id: {} type: {} fromVersion: {} toVersion: {}. " +
+                                    "Going to set default configuration ... ",
+                            node.getId(), node.getType(), configurationVersion, currentVersion, e);
+                    node.setConfiguration(getDefaultConfig(configClass));
                 }
             }
         }
-        node.setConfigurationVersion(nodeInfo.getCurrentVersion());
+        node.setConfigurationVersion(currentVersion);
     }
 
-    private static boolean isValidConfig(JsonNode oldConfiguration, Class<? extends NodeConfiguration> configClass) {
-        try {
-            JacksonUtil.treeToValue(oldConfiguration, configClass);
-            return true;
-        } catch (Exception e) {
-            return false;
-        }
+    @SneakyThrows
+    private static TbNode getTbVersionedNode(RuleNodeClassInfo nodeInfo) {
+        return (TbNode) nodeInfo.getClazz().getDeclaredConstructor().newInstance();
+    }
+
+    @SneakyThrows
+    private static JsonNode getDefaultConfig(Class<? extends NodeConfiguration> configClass) {
+        return JacksonUtil.valueToTree(configClass.getDeclaredConstructor().newInstance().defaultConfiguration());
     }
 
 }

application/src/test/java/org/thingsboard/server/utils/TbNodeUpgradeUtilsTest.java

@@ -40,7 +40,7 @@ public class TbNodeUpgradeUtilsTest {
         var annotation = mock(org.thingsboard.rule.engine.api.RuleNode.class);
         var defaultConfig = JacksonUtil.valueToTree(nodeConfigClazz.getDeclaredConstructor().newInstance().defaultConfiguration());
 
-        when(nodeInfo.getClazz()).thenReturn((Class)TbGetAttributesNode.class);
+        when(nodeInfo.getClazz()).thenReturn((Class) TbGetAttributesNode.class);
         when(nodeInfo.getCurrentVersion()).thenReturn(1);
         when(nodeInfo.getAnnotation()).thenReturn(annotation);
         when(annotation.configClazz()).thenReturn((Class) nodeConfigClazz);
@@ -62,7 +62,7 @@ public class TbNodeUpgradeUtilsTest {
         var annotation = mock(org.thingsboard.rule.engine.api.RuleNode.class);
         var defaultConfig = JacksonUtil.valueToTree(nodeConfigClazz.getDeclaredConstructor().newInstance().defaultConfiguration());
 
-        when(nodeInfo.getClazz()).thenReturn((Class)TbGetAttributesNode.class);
+        when(nodeInfo.getClazz()).thenReturn((Class) TbGetAttributesNode.class);
         when(nodeInfo.getCurrentVersion()).thenReturn(1);
         when(nodeInfo.getAnnotation()).thenReturn(annotation);
         when(annotation.configClazz()).thenReturn((Class) nodeConfigClazz);
@@ -83,7 +83,7 @@ public class TbNodeUpgradeUtilsTest {
         var annotation = mock(org.thingsboard.rule.engine.api.RuleNode.class);
         var defaultConfig = JacksonUtil.valueToTree(nodeConfigClazz.getDeclaredConstructor().newInstance().defaultConfiguration());
 
-        when(nodeInfo.getClazz()).thenReturn((Class)TbGetAttributesNode.class);
+        when(nodeInfo.getClazz()).thenReturn((Class) TbGetAttributesNode.class);
         when(nodeInfo.getCurrentVersion()).thenReturn(1);
         when(nodeInfo.getAnnotation()).thenReturn(annotation);
         when(annotation.configClazz()).thenReturn((Class) nodeConfigClazz);
@@ -130,4 +130,29 @@ public class TbNodeUpgradeUtilsTest {
 
     }
 
+    @Test
+    public void testUpgradeRuleNodeConfigurationWithInvalidConfigAndOldConfigVersion() throws Exception {
+        // GIVEN
+        var node = new RuleNode();
+        var nodeInfo = mock(RuleNodeClassInfo.class);
+        var nodeConfigClazz = TbGetEntityDataNodeConfiguration.class;
+        var annotation = mock(org.thingsboard.rule.engine.api.RuleNode.class);
+        var defaultConfig = JacksonUtil.valueToTree(nodeConfigClazz.getDeclaredConstructor().newInstance().defaultConfiguration());
+
+        when(nodeInfo.getClazz()).thenReturn((Class) TbGetCustomerAttributeNode.class);
+        when(nodeInfo.getCurrentVersion()).thenReturn(1);
+        when(nodeInfo.getAnnotation()).thenReturn(annotation);
+        when(annotation.configClazz()).thenReturn((Class) nodeConfigClazz);
+
+        // missing telemetry field
+        String oldConfig = "{\"attrMapping\":{\"alarmThreshold\":\"threshold\"}}";;
+        node.setConfiguration(JacksonUtil.toJsonNode(oldConfig));
+        // WHEN
+        TbNodeUpgradeUtils.upgradeConfigurationAndVersion(node, nodeInfo);
+        // THEN
+        Assertions.assertThat(node.getConfiguration()).isEqualTo(defaultConfig);
+        Assertions.assertThat(node.getConfigurationVersion()).isEqualTo(1);
+
+    }
+
 }