diff --git a/application/src/main/data/json/edge/rule_chains/edge_root_rule_chain.json b/application/src/main/data/json/edge/rule_chains/edge_root_rule_chain.json index 790c9d36c7..6b7603c026 100644 --- a/application/src/main/data/json/edge/rule_chains/edge_root_rule_chain.json +++ b/application/src/main/data/json/edge/rule_chains/edge_root_rule_chain.json @@ -48,12 +48,12 @@ "type": "org.thingsboard.rule.engine.telemetry.TbMsgAttributesNode", "name": "Save Client Attributes", "debugMode": false, - "configurationVersion": 1, + "configurationVersion": 2, "configuration": { "scope": "CLIENT_SCOPE", - "notifyDevice": "false", - "sendAttributesUpdatedNotification": "false", - "updateAttributesOnlyOnValueChange": "true" + "notifyDevice": false, + "sendAttributesUpdatedNotification": false, + "updateAttributesOnlyOnValueChange": true }, "externalId": null }, diff --git a/application/src/main/data/json/tenant/device_profile/rule_chain_template.json b/application/src/main/data/json/tenant/device_profile/rule_chain_template.json index d03fcd0b7b..0256a2ccf2 100644 --- a/application/src/main/data/json/tenant/device_profile/rule_chain_template.json +++ b/application/src/main/data/json/tenant/device_profile/rule_chain_template.json @@ -32,12 +32,12 @@ "type": "org.thingsboard.rule.engine.telemetry.TbMsgAttributesNode", "name": "Save Client Attributes", "debugMode": false, - "configurationVersion": 1, + "configurationVersion": 2, "configuration": { "scope": "CLIENT_SCOPE", - "notifyDevice": "false", - "sendAttributesUpdatedNotification": "false", - "updateAttributesOnlyOnValueChange": "true" + "notifyDevice": false, + "sendAttributesUpdatedNotification": false, + "updateAttributesOnlyOnValueChange": true } }, { diff --git a/application/src/main/data/json/tenant/rule_chains/root_rule_chain.json b/application/src/main/data/json/tenant/rule_chains/root_rule_chain.json index 419d30ed4c..0b70d087e7 100644 --- a/application/src/main/data/json/tenant/rule_chains/root_rule_chain.json +++ b/application/src/main/data/json/tenant/rule_chains/root_rule_chain.json @@ -31,12 +31,12 @@ "type": "org.thingsboard.rule.engine.telemetry.TbMsgAttributesNode", "name": "Save Client Attributes", "debugMode": false, - "configurationVersion": 1, + "configurationVersion": 2, "configuration": { "scope": "CLIENT_SCOPE", - "notifyDevice": "false", - "sendAttributesUpdatedNotification": "false", - "updateAttributesOnlyOnValueChange": "true" + "notifyDevice": false, + "sendAttributesUpdatedNotification": false, + "updateAttributesOnlyOnValueChange": true } }, { diff --git a/application/src/main/data/upgrade/3.6.1/save_attributes_node_update.sql b/application/src/main/data/upgrade/3.6.1/save_attributes_node_update.sql new file mode 100644 index 0000000000..6a27c8ebc2 --- /dev/null +++ b/application/src/main/data/upgrade/3.6.1/save_attributes_node_update.sql @@ -0,0 +1,26 @@ +-- +-- Copyright © 2016-2023 The Thingsboard Authors +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- + +UPDATE rule_node SET + configuration = (configuration::jsonb || jsonb_build_object( + 'notifyDevice', + CASE WHEN configuration::jsonb ->> 'notifyDevice' = 'false' THEN false ELSE true END, + 'sendAttributesUpdatedNotification', + CASE WHEN configuration::jsonb ->> 'sendAttributesUpdatedNotification' = 'true' THEN true ELSE false END, + 'updateAttributesOnlyOnValueChange', + CASE WHEN configuration::jsonb ->> 'updateAttributesOnlyOnValueChange' = 'false' THEN false ELSE true END)::jsonb)::varchar, + configuration_version = 2 +WHERE type = 'org.thingsboard.rule.engine.telemetry.TbMsgAttributesNode' AND configuration_version = 1; diff --git a/application/src/main/java/org/thingsboard/server/service/install/SqlDatabaseUpgradeService.java b/application/src/main/java/org/thingsboard/server/service/install/SqlDatabaseUpgradeService.java index a4e9d61a74..f404edb8b1 100644 --- a/application/src/main/java/org/thingsboard/server/service/install/SqlDatabaseUpgradeService.java +++ b/application/src/main/java/org/thingsboard/server/service/install/SqlDatabaseUpgradeService.java @@ -759,7 +759,14 @@ public class SqlDatabaseUpgradeService implements DatabaseEntitiesUpgradeService updateSchema("3.6.0", 3006000, "3.6.1", 3006001, null); break; case "3.6.1": - updateSchema("3.6.1", 3006001, "3.6.2", 3006002, null); + updateSchema("3.6.1", 3006001, "3.6.2", 3006002, connection -> { + try { + Path saveAttributesNodeUpdateFile = Paths.get(installScripts.getDataDir(), "upgrade", "3.6.1", "save_attributes_node_update.sql"); + loadSql(saveAttributesNodeUpdateFile, connection); + } catch (Exception e) { + log.warn("Failed to execute update script for save attributes rule nodes due to: ", e); + } + }); break; default: throw new RuntimeException("Unable to upgrade SQL database, unsupported fromVersion: " + fromVersion); diff --git a/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNode.java b/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNode.java index 06498241fb..b2df498fb6 100644 --- a/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNode.java +++ b/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNode.java @@ -28,13 +28,13 @@ import org.thingsboard.rule.engine.api.TbNode; import org.thingsboard.rule.engine.api.TbNodeConfiguration; import org.thingsboard.rule.engine.api.TbNodeException; import org.thingsboard.rule.engine.api.util.TbNodeUtils; +import org.thingsboard.server.common.adaptor.JsonConverter; import org.thingsboard.server.common.data.StringUtils; import org.thingsboard.server.common.data.kv.AttributeKvEntry; import org.thingsboard.server.common.data.kv.KvEntry; import org.thingsboard.server.common.data.plugin.ComponentType; import org.thingsboard.server.common.data.util.TbPair; import org.thingsboard.server.common.msg.TbMsg; -import org.thingsboard.server.common.adaptor.JsonConverter; import java.util.ArrayList; import java.util.List; @@ -43,7 +43,9 @@ import java.util.Objects; import java.util.function.Function; import java.util.stream.Collectors; -import static org.thingsboard.server.common.data.DataConstants.*; +import static org.thingsboard.server.common.data.DataConstants.CLIENT_SCOPE; +import static org.thingsboard.server.common.data.DataConstants.NOTIFY_DEVICE_METADATA_KEY; +import static org.thingsboard.server.common.data.DataConstants.SCOPE; import static org.thingsboard.server.common.data.msg.TbMsgType.POST_ATTRIBUTES_REQUEST; @Slf4j @@ -51,7 +53,7 @@ import static org.thingsboard.server.common.data.msg.TbMsgType.POST_ATTRIBUTES_R type = ComponentType.ACTION, name = "save attributes", configClazz = TbMsgAttributesNodeConfiguration.class, - version = 1, + version = 2, nodeDescription = "Saves attributes data", nodeDetails = "Saves entity attributes based on configurable scope parameter. Expects messages with 'POST_ATTRIBUTES_REQUEST' message type. " + "If upsert(update/insert) operation is completed successfully rule node will send the incoming message via Success chain, otherwise, Failure chain is used. " + @@ -64,15 +66,15 @@ import static org.thingsboard.server.common.data.msg.TbMsgType.POST_ATTRIBUTES_R ) public class TbMsgAttributesNode implements TbNode { + static final String NOTIFY_DEVICE_KEY = "notifyDevice"; + static final String SEND_ATTRIBUTES_UPDATED_NOTIFICATION_KEY = "sendAttributesUpdatedNotification"; static final String UPDATE_ATTRIBUTES_ONLY_ON_VALUE_CHANGE_KEY = "updateAttributesOnlyOnValueChange"; + private TbMsgAttributesNodeConfiguration config; @Override public void init(TbContext ctx, TbNodeConfiguration configuration) throws TbNodeException { this.config = TbNodeUtils.convert(configuration, TbMsgAttributesNodeConfiguration.class); - if (config.getNotifyDevice() == null) { - config.setNotifyDevice(true); - } } @Override @@ -117,7 +119,7 @@ public class TbMsgAttributesNode implements TbNode { msg.getOriginator(), scope, attributes, - checkNotifyDevice(msg.getMetaData().getValue(NOTIFY_DEVICE_METADATA_KEY)), + config.isNotifyDevice() || checkNotifyDeviceMdValue(msg.getMetaData().getValue(NOTIFY_DEVICE_METADATA_KEY)), sendAttributesUpdateNotification ? new AttributesUpdateNodeCallback(ctx, msg, scope, attributes) : new TelemetryNodeCallback(ctx, msg) @@ -146,8 +148,9 @@ public class TbMsgAttributesNode implements TbNode { return config.isSendAttributesUpdatedNotification() && !CLIENT_SCOPE.equals(scope); } - private boolean checkNotifyDevice(String notifyDeviceMdValue) { - return config.getNotifyDevice() || StringUtils.isEmpty(notifyDeviceMdValue) || Boolean.parseBoolean(notifyDeviceMdValue); + private boolean checkNotifyDeviceMdValue(String notifyDeviceMdValue) { + // Check for empty string for backward-compatibility. A while ago node always notified devices. + return StringUtils.isEmpty(notifyDeviceMdValue) || Boolean.parseBoolean(notifyDeviceMdValue); } private String getScope(String mdScopeValue) { @@ -166,6 +169,13 @@ public class TbMsgAttributesNode implements TbNode { hasChanges = true; ((ObjectNode) oldConfiguration).put(UPDATE_ATTRIBUTES_ONLY_ON_VALUE_CHANGE_KEY, false); } + case 1: + // update notifyDevice. set true if null or property doesn't exist for backward-compatibility. + hasChanges = fixEscapedBooleanConfigParameter(oldConfiguration, NOTIFY_DEVICE_KEY, hasChanges, true); + // update sendAttributesUpdatedNotification. + hasChanges = fixEscapedBooleanConfigParameter(oldConfiguration, SEND_ATTRIBUTES_UPDATED_NOTIFICATION_KEY, hasChanges, false); + // update updateAttributesOnlyOnValueChange. + hasChanges = fixEscapedBooleanConfigParameter(oldConfiguration, UPDATE_ATTRIBUTES_ONLY_ON_VALUE_CHANGE_KEY, hasChanges, true); break; default: break; @@ -173,4 +183,20 @@ public class TbMsgAttributesNode implements TbNode { return new TbPair<>(hasChanges, oldConfiguration); } + private boolean fixEscapedBooleanConfigParameter(JsonNode oldConfiguration, String boolKey, boolean hasChanges, boolean valueIfNull) { + if (oldConfiguration.hasNonNull(boolKey)) { + var value = oldConfiguration.get(boolKey); + if (value.isTextual()) { + hasChanges = true; + ((ObjectNode) oldConfiguration) + .put(boolKey, value.asBoolean(valueIfNull)); + } + } else { + hasChanges = true; + ((ObjectNode) oldConfiguration) + .put(boolKey, valueIfNull); + } + return hasChanges; + } + } diff --git a/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNodeConfiguration.java b/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNodeConfiguration.java index 1dd98feb16..d8502e76d9 100644 --- a/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNodeConfiguration.java +++ b/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/telemetry/TbMsgAttributesNodeConfiguration.java @@ -24,7 +24,7 @@ public class TbMsgAttributesNodeConfiguration implements NodeConfiguration newAttributes = new ArrayList<>(); List filtered = node.filterChangedAttr(Collections.emptyList(), newAttributes); @@ -57,7 +88,6 @@ class TbMsgAttributesNodeTest { @Test void testFilterChangedAttr_whenCurrentAttributesContainsInAnyOrderNewAttributes_thenReturnEmptyList() { - TbMsgAttributesNode node = spy(TbMsgAttributesNode.class); List currentAttributes = List.of( new BaseAttributeKvEntry(1694000000L, new StringDataEntry("address", "Peremohy ave 1")), new BaseAttributeKvEntry(1694000000L, new BooleanDataEntry("valid", true)), @@ -78,7 +108,6 @@ class TbMsgAttributesNodeTest { @Test void testFilterChangedAttr_whenCurrentAttributesContainsInAnyOrderNewAttributes_thenReturnExpectedList() { - TbMsgAttributesNode node = spy(TbMsgAttributesNode.class); List currentAttributes = List.of( new BaseAttributeKvEntry(1694000000L, new StringDataEntry("address", "Peremohy ave 1")), new BaseAttributeKvEntry(1694000000L, new BooleanDataEntry("valid", true)), @@ -103,41 +132,112 @@ class TbMsgAttributesNodeTest { assertThat(filtered).containsExactlyInAnyOrderElementsOf(expected); } - @Test - void testUpgrade_fromVersion0() throws TbNodeException { + // Notify device backward-compatibility test arguments + private static Stream givenNotifyDeviceMdValue_whenSaveAndNotify_thenVerifyExpectedArgumentForNotifyDeviceInSaveAndNotifyMethod() { + return Stream.of( + Arguments.of(null, true), + Arguments.of("null", false), + Arguments.of("true", true), + Arguments.of("false", false) + ); + } - TbMsgAttributesNode node = mock(TbMsgAttributesNode.class); - willCallRealMethod().given(node).upgrade(anyInt(), any()); + // Notify device backward-compatibility test + @ParameterizedTest + @MethodSource + void givenNotifyDeviceMdValue_whenSaveAndNotify_thenVerifyExpectedArgumentForNotifyDeviceInSaveAndNotifyMethod(String mdValue, boolean expectedArgumentValue) throws TbNodeException { + var ctxMock = mock(TbContext.class); + var telemetryServiceMock = mock(RuleEngineTelemetryService.class); + ObjectNode defaultConfig = (ObjectNode) JacksonUtil.valueToTree(new TbMsgAttributesNodeConfiguration().defaultConfiguration()); + defaultConfig.put("notifyDevice", false); + var tbNodeConfiguration = new TbNodeConfiguration(defaultConfig); + + assertThat(defaultConfig.has("notifyDevice")).as("pre condition has notifyDevice").isTrue(); + + when(ctxMock.getTenantId()).thenReturn(tenantId); + when(ctxMock.getTelemetryService()).thenReturn(telemetryServiceMock); + willCallRealMethod().given(node).init(any(TbContext.class), any(TbNodeConfiguration.class)); + willCallRealMethod().given(node).saveAttr(any(), eq(ctxMock), any(TbMsg.class), anyString(), anyBoolean()); + + node.init(ctxMock, tbNodeConfiguration); - ObjectNode jsonNode = (ObjectNode) JacksonUtil.valueToTree(new TbMsgAttributesNodeConfiguration().defaultConfiguration()); - jsonNode.remove(updateAttributesOnlyOnValueChangeKey); - assertThat(jsonNode.has(updateAttributesOnlyOnValueChangeKey)).as("pre condition has no " + updateAttributesOnlyOnValueChangeKey).isFalse(); + TbMsgMetaData md = new TbMsgMetaData(); + if (mdValue != null) { + md.putValue(NOTIFY_DEVICE_METADATA_KEY, mdValue); + } + // dummy list with one ts kv to pass the empty list check. + var testTbMsg = TbMsg.newMsg(TbMsgType.POST_TELEMETRY_REQUEST, deviceId, md, TbMsg.EMPTY_STRING); + List testAttrList = List.of(new BaseAttributeKvEntry(0L, new StringDataEntry("testKey", "testValue"))); - TbPair upgradeResult = node.upgrade(0, jsonNode); + node.saveAttr(testAttrList, ctxMock, testTbMsg, DataConstants.SHARED_SCOPE, false); - ObjectNode resultNode = (ObjectNode) upgradeResult.getSecond(); - assertThat(upgradeResult.getFirst()).as("upgrade result has changes").isTrue(); - assertThat(resultNode.has(updateAttributesOnlyOnValueChangeKey)).as("upgrade result has key " + updateAttributesOnlyOnValueChangeKey).isTrue(); - assertThat(resultNode.get(updateAttributesOnlyOnValueChangeKey).asBoolean()).as("upgrade result value [false] for key " + updateAttributesOnlyOnValueChangeKey).isFalse(); + ArgumentCaptor notifyDeviceCaptor = ArgumentCaptor.forClass(Boolean.class); + + verify(telemetryServiceMock, times(1)).saveAndNotify( + eq(tenantId), eq(deviceId), eq(DataConstants.SHARED_SCOPE), + eq(testAttrList), notifyDeviceCaptor.capture(), any() + ); + boolean notifyDevice = notifyDeviceCaptor.getValue(); + assertThat(notifyDevice).isEqualTo(expectedArgumentValue); } - @Test - void testUpgrade_fromVersion0_alreadyHasupdateAttributesOnlyOnValueChange() throws TbNodeException { - TbMsgAttributesNode node = mock(TbMsgAttributesNode.class); - willCallRealMethod().given(node).upgrade(anyInt(), any()); - ObjectNode jsonNode = (ObjectNode) JacksonUtil.valueToTree(new TbMsgAttributesNodeConfiguration().defaultConfiguration()); - jsonNode.remove(updateAttributesOnlyOnValueChangeKey); - jsonNode.put(updateAttributesOnlyOnValueChangeKey, true); - assertThat(jsonNode.has(updateAttributesOnlyOnValueChangeKey)).as("pre condition has no " + updateAttributesOnlyOnValueChangeKey).isTrue(); - assertThat(jsonNode.get(updateAttributesOnlyOnValueChangeKey).asBoolean()).as("pre condition has [true] for key " + updateAttributesOnlyOnValueChangeKey).isTrue(); + // Rule nodes upgrade + private static Stream givenFromVersionAndConfig_whenUpgrade_thenVerifyHasChangesAndConfig() { + return Stream.of( + // default config for version 0 + Arguments.of(0, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":\"false\",\"sendAttributesUpdatedNotification\":\"false\"}", + true, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":false,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":false}"), + // default config for version 1 with upgrade from version 0 + Arguments.of(0, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":false,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}", + false, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":false,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}"), + // all flags are booleans + Arguments.of(1, + "{\"scope\":\"SHARED_SCOPE\",\"notifyDevice\":true,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}", + false, + "{\"scope\":\"SHARED_SCOPE\",\"notifyDevice\":true,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}"), + // no boolean flags set + Arguments.of(1, + "{\"scope\":\"CLIENT_SCOPE\"}", + true, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":true,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}"), + // all flags are boolean strings + Arguments.of(1, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":\"false\",\"sendAttributesUpdatedNotification\":\"false\",\"updateAttributesOnlyOnValueChange\":\"true\"}", + true, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":false,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}"), + // at least one flag is boolean string + Arguments.of(1, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":\"false\",\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}", + true, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":false,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}"), + // notify device flag is null + Arguments.of(1, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":\"null\",\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}", + true, + "{\"scope\":\"CLIENT_SCOPE\",\"notifyDevice\":true,\"sendAttributesUpdatedNotification\":false,\"updateAttributesOnlyOnValueChange\":true}") + ); + } + + @ParameterizedTest + @MethodSource + void givenFromVersionAndConfig_whenUpgrade_thenVerifyHasChangesAndConfig(int givenVersion, String givenConfigStr, boolean hasChanges, String expectedConfigStr) throws TbNodeException { + // GIVEN + willCallRealMethod().given(node).upgrade(anyInt(), any()); + JsonNode givenConfig = JacksonUtil.toJsonNode(givenConfigStr); + JsonNode expectedConfig = JacksonUtil.toJsonNode(expectedConfigStr); - TbPair upgradeResult = node.upgrade(0, jsonNode); + // WHEN + TbPair upgradeResult = node.upgrade(givenVersion, givenConfig); - ObjectNode resultNode = (ObjectNode) upgradeResult.getSecond(); - assertThat(upgradeResult.getFirst()).as("upgrade result has changes").isFalse(); - assertThat(resultNode.has(updateAttributesOnlyOnValueChangeKey)).as("upgrade result has key " + updateAttributesOnlyOnValueChangeKey).isTrue(); - assertThat(resultNode.get(updateAttributesOnlyOnValueChangeKey).asBoolean()).as("upgrade result value [true] for key " + updateAttributesOnlyOnValueChangeKey).isTrue(); + // THEN + assertThat(upgradeResult.getFirst()).isEqualTo(hasChanges); + ObjectNode upgradedConfig = (ObjectNode) upgradeResult.getSecond(); + assertThat(upgradedConfig).isEqualTo(expectedConfig); } }