From bf6f0f1ec5e6e0c38b0e9ac05a6faebb4a540ac1 Mon Sep 17 00:00:00 2001 From: dshvaika Date: Thu, 9 Jul 2026 14:53:42 +0300 Subject: [PATCH] Refine ENTITY_AGGREGATION CF numeric regression coverage in integration test Address review feedback on the integration test: - Read latest telemetry with useStrictDataTypes=true always, so the value node keeps its stored type (numeric -> JSON number, str_v -> JSON string). - Assert the aggregation result is a numeric node across all existing scenarios via a shared assertNumericValue helper, instead of a separate standalone test. isNumber() is the actual regression guard; asLong()/asText() would coerce a str_v string and miss it, so the type is asserted explicitly and the value is compared numerically. - Drop the redundant standalone test that duplicated an existing scenario. --- .../EntityAggregationCalculatedFieldTest.java | 85 ++++++------------- 1 file changed, 27 insertions(+), 58 deletions(-) diff --git a/application/src/test/java/org/thingsboard/server/cf/EntityAggregationCalculatedFieldTest.java b/application/src/test/java/org/thingsboard/server/cf/EntityAggregationCalculatedFieldTest.java index a51ac508be..71b89bff40 100644 --- a/application/src/test/java/org/thingsboard/server/cf/EntityAggregationCalculatedFieldTest.java +++ b/application/src/test/java/org/thingsboard/server/cf/EntityAggregationCalculatedFieldTest.java @@ -15,6 +15,7 @@ */ package org.thingsboard.server.cf; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; import org.junit.After; import org.junit.Before; @@ -106,7 +107,7 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgConsumption"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("9999"); + assertNumericValue(result, "consumption", 9999); assertThat(result.get("avgConsumption").get(0).get("value").isNull()).isTrue(); }); } @@ -137,8 +138,8 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgConsumption"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("400"); - assertThat(result.get("avgConsumption").get(0).get("value").asText()).isEqualTo("133"); + assertNumericValue(result, "consumption", 400); + assertNumericValue(result, "avgConsumption", 133); }); postTelemetry(device.getId(), String.format("{\"ts\": \"%s\", \"values\": {\"energy\":500}}", tsInInterval_1)); @@ -149,46 +150,8 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgConsumption"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("400"); - assertThat(result.get("avgConsumption").get(0).get("value").asText()).isEqualTo("133"); - }); - } - - @Test - public void testAggregationResult_isStoredAsNumericTelemetry() throws Exception { - // Regression: ENTITY_AGGREGATION must store numeric results as numbers (ts_kv.dbl_v/long_v), - // not as JSON strings (ts_kv.str_v) - otherwise server-side AVG/SUM return no data. - // The existing .asText()-based tests cannot catch this (asText coerces both types), so this - // test reads with useStrictDataTypes=true and asserts the value node type. - Device device = createDevice("Device", "1234567890111"); - - CustomInterval customInterval = new CustomInterval(TZ, 0L, 5L); - createConsumptionCF(device.getId(), customInterval, null); - - long currentIntervalStartTs = customInterval.getCurrentIntervalStartTs(); - long tsInInterval_1 = currentIntervalStartTs + 1000; - long tsInInterval_2 = currentIntervalStartTs + 500; - long tsInInterval_3 = currentIntervalStartTs + 200; - postTelemetry(device.getId(), String.format("{\"ts\": \"%s\", \"values\": {\"energy\":100}}", tsInInterval_1)); - postTelemetry(device.getId(), String.format("{\"ts\": \"%s\", \"values\": {\"energy\":180}}", tsInInterval_2)); - postTelemetry(device.getId(), String.format("{\"ts\": \"%s\", \"values\": {\"energy\":120}}", tsInInterval_3)); - - long interval = customInterval.getCurrentIntervalDurationMillis(); - - await().alias("create CF -> aggregation result stored as numeric telemetry") - .atMost(2 * interval, TimeUnit.MILLISECONDS) - .pollInterval(POLL_INTERVAL, TimeUnit.SECONDS) - .untilAsserted(() -> { - ObjectNode result = getLatestTelemetryStrict(device.getId(), "consumption", "avgConsumption"); - assertThat(result).isNotNull(); - assertThat(result.get("consumption")).isNotNull(); - assertThat(result.get("avgConsumption")).isNotNull(); - // SUM and AVG results must be numeric JSON nodes, not strings. - assertThat(result.get("consumption").get(0).get("value").isNumber()).isTrue(); - assertThat(result.get("avgConsumption").get(0).get("value").isNumber()).isTrue(); - // Values are still correct (SUM=400, AVG=133). - assertThat(result.get("consumption").get(0).get("value").asInt()).isEqualTo(400); - assertThat(result.get("avgConsumption").get(0).get("value").asInt()).isEqualTo(133); + assertNumericValue(result, "consumption", 400); + assertNumericValue(result, "avgConsumption", 133); }); } @@ -219,8 +182,8 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgConsumption"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("400"); - assertThat(result.get("avgConsumption").get(0).get("value").asText()).isEqualTo("133"); + assertNumericValue(result, "consumption", 400); + assertNumericValue(result, "avgConsumption", 133); }); postTelemetry(device.getId(), String.format("{\"ts\": \"%s\", \"values\": {\"energy\":300}}", tsInInterval_1)); @@ -231,8 +194,8 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgConsumption"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("600"); - assertThat(result.get("avgConsumption").get(0).get("value").asText()).isEqualTo("200"); + assertNumericValue(result, "consumption", 600); + assertNumericValue(result, "avgConsumption", 200); }); } @@ -269,8 +232,8 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgConsumption"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("400"); - assertThat(result.get("avgConsumption").get(0).get("value").asText()).isEqualTo("133"); + assertNumericValue(result, "consumption", 400); + assertNumericValue(result, "avgConsumption", 133); }); postTelemetry(device.getId(), String.format("{\"ts\": \"%s\", \"values\": {\"energy\":500}}", currentIntervalStartTs + 4500L)); @@ -281,9 +244,9 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgConsumption"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("500"); + assertNumericValue(result, "consumption", 500); assertThat(result.get("consumption").get(0).get("ts").asLong()).isEqualTo(currentIntervalStartTs + 4000L); - assertThat(result.get("avgConsumption").get(0).get("value").asText()).isEqualTo("500"); + assertNumericValue(result, "avgConsumption", 500); assertThat(result.get("avgConsumption").get(0).get("ts").asLong()).isEqualTo(currentIntervalStartTs + 4000L); }); } @@ -344,8 +307,8 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest .untilAsserted(() -> { ObjectNode result = getLatestTelemetry(device.getId(), "consumption", "avgTemperature"); assertThat(result).isNotNull(); - assertThat(result.get("consumption").get(0).get("value").asText()).isEqualTo("400"); - assertThat(result.get("avgTemperature").get(0).get("value").asText()).isEqualTo("39"); + assertNumericValue(result, "consumption", 400); + assertNumericValue(result, "avgTemperature", 39); }); } @@ -410,14 +373,20 @@ public class EntityAggregationCalculatedFieldTest extends AbstractControllerTest return saveCalculatedField(calculatedField); } - private ObjectNode getLatestTelemetry(EntityId entityId, String... keys) throws Exception { - return doGetAsync("/api/plugins/telemetry/" + entityId.getEntityType() + "/" + entityId.getId() + "/values/timeseries?keys=" + String.join(",", keys), ObjectNode.class); - } - // useStrictDataTypes=true so the value node keeps its stored type (numeric -> JSON number, str_v -> JSON string). // Without it the endpoint returns every value via getValueAsString(), masking the string-vs-number distinction. - private ObjectNode getLatestTelemetryStrict(EntityId entityId, String... keys) throws Exception { + private ObjectNode getLatestTelemetry(EntityId entityId, String... keys) throws Exception { return doGetAsync("/api/plugins/telemetry/" + entityId.getEntityType() + "/" + entityId.getId() + "/values/timeseries?useStrictDataTypes=true&keys=" + String.join(",", keys), ObjectNode.class); } + // Regression guard: a numeric aggregation result must be stored as a numeric JSON node (ts_kv.dbl_v/long_v), + // not a JSON string (ts_kv.str_v) - otherwise server-side AVG/SUM return no data. A value-only check would + // not catch this: asLong()/asText() coerce a string node like "400" to the same value/text, so the node type + // is asserted explicitly; the numeric comparison then verifies the aggregated value. + private static void assertNumericValue(ObjectNode result, String key, long expectedValue) { + JsonNode value = result.get(key).get(0).get("value"); + assertThat(value.isNumber()).as(key + " should be stored as a numeric node").isTrue(); + assertThat(value.asLong()).isEqualTo(expectedValue); + } + }