From 9279741a4ef38cb89da4c0624682ec57ac6f80a6 Mon Sep 17 00:00:00 2001 From: Remco Westerhoud Date: Mon, 11 Apr 2022 17:15:49 +0200 Subject: [PATCH 1/2] fix: NPE during logging Variable with a value of null would produce a NPE during the logging of the record stream. This nullcheck fixes this issue. --- .../test/filters/logger/RecordStreamLogger.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java b/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java index 6bf35b36..20924764 100644 --- a/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java +++ b/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java @@ -53,7 +53,7 @@ public class RecordStreamLogger { private final Map, String>> valueTypeLoggers = new HashMap<>(); public RecordStreamLogger(final RecordStreamSource recordStreamSource) { - this.recordStream = RecordStream.of(recordStreamSource); + recordStream = RecordStream.of(recordStreamSource); valueTypeLoggers.put(ValueType.JOB, this::logJobRecordValue); valueTypeLoggers.put(ValueType.DEPLOYMENT, this::logDeploymentRecordValue); valueTypeLoggers.put(ValueType.PROCESS_INSTANCE, this::logProcessInstanceRecordValue); @@ -272,7 +272,14 @@ private String logVariables(final Map variables) { } final StringJoiner joiner = new StringJoiner(", ", "[", "]"); - variables.forEach((key, value) -> joiner.add(key + " -> " + value.toString())); + variables.forEach( + (key, value) -> { + if (value != null) { + joiner.add(key + " -> " + value); + } else { + joiner.add(key + " -> null"); + } + }); return String.format("(Variables: %s)", joiner); } From ba7227d30751dc4c3c2dfcb564ba98c5fe8ff641 Mon Sep 17 00:00:00 2001 From: Remco Westerhoud Date: Mon, 11 Apr 2022 17:17:04 +0200 Subject: [PATCH 2/2] test: add tests for RecordStreamLogger This class was mostly not tested. We tested if we had a logger for all records, but we did not test the fucntionality of the logger itself. It doesn't contain much logic but by adding some tests around it we can hopefully prevent NPE's in the future. --- filters/pom.xml | 5 ++ .../filters/logger/RecordStreamLogger.java | 11 +---- .../logger/RecordStreamLoggerTest.java | 48 +++++++++++++++++++ .../AbstractProcessInstanceAssertTest.java | 1 + 4 files changed, 56 insertions(+), 9 deletions(-) diff --git a/filters/pom.xml b/filters/pom.xml index c6846137..8e243a61 100644 --- a/filters/pom.xml +++ b/filters/pom.xml @@ -55,6 +55,11 @@ test + + org.junit.jupiter + junit-jupiter-params + test + diff --git a/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java b/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java index 20924764..ba73a539 100644 --- a/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java +++ b/filters/src/main/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLogger.java @@ -266,20 +266,13 @@ private String logProcessEventRecordValue(final Record record) { return joiner.toString(); } - private String logVariables(final Map variables) { + protected String logVariables(final Map variables) { if (variables.isEmpty()) { return ""; } final StringJoiner joiner = new StringJoiner(", ", "[", "]"); - variables.forEach( - (key, value) -> { - if (value != null) { - joiner.add(key + " -> " + value); - } else { - joiner.add(key + " -> null"); - } - }); + variables.forEach((key, value) -> joiner.add(key + " -> " + value)); return String.format("(Variables: %s)", joiner); } diff --git a/filters/src/test/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLoggerTest.java b/filters/src/test/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLoggerTest.java index ba111127..a09db818 100644 --- a/filters/src/test/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLoggerTest.java +++ b/filters/src/test/java/io/camunda/zeebe/process/test/filters/logger/RecordStreamLoggerTest.java @@ -15,16 +15,34 @@ */ package io.camunda.zeebe.process.test.filters.logger; +import static org.assertj.core.api.Assertions.assertThat; + import io.camunda.zeebe.protocol.record.Record; import io.camunda.zeebe.protocol.record.ValueType; import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.function.Function; +import java.util.stream.Stream; import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class RecordStreamLoggerTest { + private static final Map TYPED_TEST_VARIABLES = new HashMap<>(); + + static { + TYPED_TEST_VARIABLES.put("stringProperty", "stringValue"); + TYPED_TEST_VARIABLES.put("numberProperty", 123); + TYPED_TEST_VARIABLES.put("booleanProperty", true); + TYPED_TEST_VARIABLES.put("complexProperty", Arrays.asList("Element 1", "Element 2")); + TYPED_TEST_VARIABLES.put("nullProperty", null); + } + @Test void testAllValueTypesAreMapped() { final Map, String>> valueTypeLoggers = @@ -41,4 +59,34 @@ void testAllValueTypesAreMapped() { softly.assertAll(); } + + @ParameterizedTest(name = "{0}") + @MethodSource("provideVariables") + void testLogVariable(final String key, final Object value) { + final RecordStreamLogger logger = new RecordStreamLogger(null); + + final String result = logger.logVariables(Collections.singletonMap(key, value)); + + assertThat(result).isEqualTo(String.format("(Variables: [%s -> %s])", key, value)); + } + + @Test + void testLogMultipleVariables() { + final RecordStreamLogger logger = new RecordStreamLogger(null); + + final String result = logger.logVariables(TYPED_TEST_VARIABLES); + + assertThat(result) + .contains("Variables: ") + .contains("stringProperty -> stringValue") + .contains("numberProperty -> 123") + .contains("booleanProperty -> true") + .contains("complexProperty -> [Element 1, Element 2]") + .contains("nullProperty -> null"); + } + + private static Stream provideVariables() { + return TYPED_TEST_VARIABLES.entrySet().stream() + .map(entry -> Arguments.of(entry.getKey(), entry.getValue())); + } } diff --git a/qa/src/test/java/io/camunda/zeebe/process/test/qa/abstracts/assertions/AbstractProcessInstanceAssertTest.java b/qa/src/test/java/io/camunda/zeebe/process/test/qa/abstracts/assertions/AbstractProcessInstanceAssertTest.java index 93c52756..4fcb7594 100644 --- a/qa/src/test/java/io/camunda/zeebe/process/test/qa/abstracts/assertions/AbstractProcessInstanceAssertTest.java +++ b/qa/src/test/java/io/camunda/zeebe/process/test/qa/abstracts/assertions/AbstractProcessInstanceAssertTest.java @@ -50,6 +50,7 @@ public abstract class AbstractProcessInstanceAssertTest { TYPED_TEST_VARIABLES.put("numberProperty", 123); TYPED_TEST_VARIABLES.put("booleanProperty", true); TYPED_TEST_VARIABLES.put("complexProperty", Arrays.asList("Element 1", "Element 2")); + TYPED_TEST_VARIABLES.put("nullProperty", null); } // These tests are for testing assertions as well as examples for users