diff --git a/horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/AlertingServiceImpl.java b/horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/AlertingServiceImpl.java index ef862f6ea..aeb17d638 100644 --- a/horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/AlertingServiceImpl.java +++ b/horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/AlertingServiceImpl.java @@ -736,12 +736,13 @@ public void updateVariables(int testId, List variablesDTO) { List variables = variablesDTO.stream().map(VariableMapper::to).collect(Collectors.toList()); List currentVariables = VariableDAO.list("testId", testId); updateCollection(currentVariables, variables, v -> v.id, item -> { - if (item.id != null && item.id <= 0) { + if (item.id != null && item.id < 0) { item.id = null; } if (item.changeDetection != null) { ensureDefaults(item.changeDetection); item.changeDetection.forEach(rd -> rd.variable = item); + item.changeDetection.stream().filter(rd -> rd.id != null && rd.id == -1).forEach(rd -> rd.id = null); } item.testId = testId; item.persist(); // insert @@ -754,7 +755,7 @@ public void updateVariables(int testId, List variablesDTO) { ensureDefaults(matching.changeDetection); } updateCollection(current.changeDetection, matching.changeDetection, rd -> rd.id, item -> { - if (item.id != null && item.id <= 0) { + if (item.id != null && item.id < 0) { item.id = null; } item.variable = current; diff --git a/horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/AlertingServiceTest.java b/horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/AlertingServiceTest.java index d34547861..09c86239b 100644 --- a/horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/AlertingServiceTest.java +++ b/horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/AlertingServiceTest.java @@ -1,35 +1,22 @@ package io.hyperfoil.tools.horreum.svc; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; - import java.io.IOException; import java.time.Instant; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; +import java.util.*; import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import java.util.stream.IntStream; -import io.hyperfoil.tools.horreum.api.alerting.ChangeDetection; -import io.hyperfoil.tools.horreum.api.alerting.DataPoint; -import io.hyperfoil.tools.horreum.api.alerting.RunExpectation; +import io.hyperfoil.tools.horreum.api.alerting.*; import io.hyperfoil.tools.horreum.api.data.Dataset; import io.hyperfoil.tools.horreum.api.data.Fingerprints; +import io.hyperfoil.tools.horreum.changedetection.RelativeDifferenceChangeDetectionModel; import io.hyperfoil.tools.horreum.bus.MessageBusChannels; import io.restassured.common.mapper.TypeRef; import jakarta.inject.Inject; -import io.hyperfoil.tools.horreum.api.alerting.Change; import io.hyperfoil.tools.horreum.api.data.Extractor; import io.hyperfoil.tools.horreum.api.data.Label; import io.hyperfoil.tools.horreum.api.data.Schema; @@ -64,6 +51,8 @@ import io.quarkus.test.oidc.server.OidcWiremockTestResource; import io.restassured.RestAssured; +import static org.junit.jupiter.api.Assertions.*; + @QuarkusTest @QuarkusTestResource(PostgresResource.class) @QuarkusTestResource(OidcWiremockTestResource.class) @@ -767,6 +756,43 @@ public void testFindLastDatapoints(TestInfo info) throws IOException { .then().statusCode(200).extract().body().as(new ParameterizedTypeImpl(List.class, AlertingService.DatapointLastTimestamp.class)); } + @org.junit.jupiter.api.Test + public void testUpdateVariablesHandlesNegativeId(TestInfo info) throws Exception { + Test test = createTest(createExampleTest(getTestName(info))); + Schema schema = createExampleSchema(info); + ChangeDetection cd = addChangeDetectionVariable(test, schema.id); + + cd.config.put("filter", "min"); + setTestVariables(test, "Value", new Label("value", schema.id), cd);//create + + List variables = variables(test.id); + assertEquals(1, variables.size()); + assertNotNull(variables.get(0).id); + + ChangeDetection problematicChangeDetection = new ChangeDetection(); + problematicChangeDetection.id = -1; //UI typically sets this value + problematicChangeDetection.model = RelativeDifferenceChangeDetectionModel.NAME; + problematicChangeDetection.config = JsonNodeFactory.instance.objectNode().put("threshold", 0.2).put("minPrevious", 2).put("window", 2).put("filter", "mean"); + List labels = Collections.singletonList("foobar"); + Set cdSet = Collections.singleton(problematicChangeDetection); + + Variable throughput = new Variable(); + throughput.id = -1; //UI typically sets this value + throughput.testId = test.id; + throughput.name = "throughput"; + throughput.labels = labels; + throughput.changeDetection = cdSet; + variables.add(throughput); + + updateVariables(test.id, variables);//update + List updated = variables(test.id); + Variable updatedThroughput = updated.stream().filter(v -> v.name.equals(throughput.name)).findFirst().get(); + assertNotEquals(-1, updatedThroughput.id); + assertEquals(throughput.changeDetection.size(), updatedThroughput.changeDetection.size()); + ChangeDetection updatedChangeDetection = updatedThroughput.changeDetection.stream().findFirst().get(); + assertNotEquals(-1, updatedChangeDetection.id); + } + private void checkChanges(Test test) { List list = ChangeDAO.list("variable.testId", test.id); assertEquals(Arrays.asList(1L, 4L, 6L, 7L, 9L),