From 68f4cf0b95c8cc23a5f3b9aeb1085c02e0ad8441 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:50:05 +0100 Subject: [PATCH] Change example rule detection used for template testing Changed to use underscore instead of hyphen as hyphens make things awkward when templating. Also added dup check for detection value names and warning. --- .../analytics/impl/AnalyticsServiceImpl.java | 12 +++-- .../impl/RuleEmailTemplatingService.java | 34 +++++++++---- .../impl/TestAnalyticsServiceImpl.java | 44 +++++++++++++++++ .../impl/TestRuleEmailTemplatingService.java | 48 +++++++++++++++---- unreleased_changes/20240919_153021_934__0.md | 19 ++++++++ 5 files changed, 133 insertions(+), 24 deletions(-) create mode 100644 stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestAnalyticsServiceImpl.java create mode 100644 unreleased_changes/20240919_153021_934__0.md diff --git a/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/AnalyticsServiceImpl.java b/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/AnalyticsServiceImpl.java index 0f0ad387f99..61041fb4feb 100644 --- a/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/AnalyticsServiceImpl.java +++ b/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/AnalyticsServiceImpl.java @@ -27,12 +27,14 @@ public void sendTestEmail(final NotificationEmailDestination emailDestination) { emailSender.send(emailDestination, getExampleDetection()); } - private Detection getExampleDetection() { + // pkg private for testing + Detection getExampleDetection() { final String stroom = "Test Environment"; final String executionTime = "2024-02-29T17:48:40.396Z"; final String detectTime = "2024-02-29T17:48:41.582Z"; final String effectiveExecutionTime = "2024-02-29T16:00:00.000Z"; + // NOTE variables (including keys in maps) cannot use '-' return Detection.builder() .withDetectTime(detectTime) .withDetectorName("Example detector for test use.") @@ -47,10 +49,10 @@ private Detection getExampleDetection() { .withExecutionTime(executionTime) .withExecutionSchedule("1hr") .withEffectiveExecutionTime(effectiveExecutionTime) - .addValue("name-1", "value-A") - .addValue("name-2", "value-B") - .addValue("name-3", "value-C") - .addValue("name-3", null) + .addValue("name_1", "value_A") + .addValue("name_2", "value_B") + .addValue("name_3", "value_C") + .addValue("name_4", null) .addLinkedEvents(new DetectionLinkedEvent(stroom, 1001L, 1L)) .addLinkedEvents(new DetectionLinkedEvent(stroom, 1001L, 2L)) .addLinkedEvents(new DetectionLinkedEvent(stroom, 2001L, 1L)) diff --git a/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/RuleEmailTemplatingService.java b/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/RuleEmailTemplatingService.java index c79033a75f5..2e94fcb6a57 100644 --- a/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/RuleEmailTemplatingService.java +++ b/stroom-analytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/RuleEmailTemplatingService.java @@ -13,9 +13,11 @@ import org.jetbrains.annotations.NotNull; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; public class RuleEmailTemplatingService { @@ -121,15 +123,29 @@ private Map buildContext(final Detection detection) { NullSafe.consume(detection.getDefunct(), val -> context.put("defunct", val)); NullSafe.consume(detection.getValues(), values -> { - // Collectors.toMap() doesn't like null values, shame - final Map map = new HashMap<>(values.size()); - values.stream() - .filter(Objects::nonNull) - .filter(detectionValue -> detectionValue.getName() != null) - .forEach(detectionValue -> - map.put(detectionValue.getName(), detectionValue.getValue())); - if (!map.isEmpty()) { - context.put("values", map); + if (!values.isEmpty()) { + // Collectors.toMap() doesn't like null values, shame + final Map map = new HashMap<>(values.size()); + final Set dupKeys = new HashSet<>(); + values.stream() + .filter(Objects::nonNull) + .filter(detectionValue -> detectionValue.getName() != null) + .forEach(detectionValue -> { + final String key = detectionValue.getName(); + if (map.containsKey(key)) { + dupKeys.add(key); + } else { + map.put(detectionValue.getName(), detectionValue.getValue()); + } + }); + if (!map.isEmpty()) { + context.put("values", map); + } + if (!dupKeys.isEmpty()) { + LOGGER.warn("Duplicate names {} found in detection values for detector '{}' ({}). " + + "The first value will be used in each case.", + dupKeys, detection.getDetectorName(), detection.getDetectorUuid()); + } } }); NullSafe.consume( diff --git a/stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestAnalyticsServiceImpl.java b/stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestAnalyticsServiceImpl.java new file mode 100644 index 00000000000..422e603d30b --- /dev/null +++ b/stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestAnalyticsServiceImpl.java @@ -0,0 +1,44 @@ +package stroom.analytics.impl; + +import stroom.ui.config.shared.AnalyticUiDefaultConfig; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.assertj.core.api.Assertions.assertThat; + +@ExtendWith(MockitoExtension.class) +class TestAnalyticsServiceImpl { + + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(TestAnalyticsServiceImpl.class); + + @Mock + private EmailSender mockEmailSender; + + final RuleEmailTemplatingService templatingService = new RuleEmailTemplatingService(); + + /** + * Verify that our example template from config works with our example detection + */ + @Test + void testExampleDetection() { + final AnalyticsServiceImpl analyticsService = new AnalyticsServiceImpl( + mockEmailSender, + templatingService); + + final Detection exampleDetection = analyticsService.getExampleDetection(); + final String template = new AnalyticUiDefaultConfig().getDefaultBodyTemplate(); + final String output = templatingService.renderTemplate(exampleDetection, template); + + LOGGER.info("output:\n{}", output); + + assertThat(output) + .contains("
  • name_1: value_A
    • ") + .contains("
  • name_4:
    • ") + .contains("
  • Environment: Test Environment, Stream ID: 1001, Event ID: 2
    • "); + } +} diff --git a/stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestRuleEmailTemplatingService.java b/stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestRuleEmailTemplatingService.java index 1711d4f2330..f01d2fc2e0f 100644 --- a/stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestRuleEmailTemplatingService.java +++ b/stroom-analytics/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestRuleEmailTemplatingService.java @@ -43,13 +43,14 @@ void renderTemplate() { void renderDefaultTemplate() { final RuleEmailTemplatingService templatingService = new RuleEmailTemplatingService(); final Detection detection = getExampleDetection(true, true); - @SuppressWarnings("checkstyle:LineLength") final String template = new AnalyticUiDefaultConfig() - .getDefaultBodyTemplate(); + final String template = new AnalyticUiDefaultConfig().getDefaultBodyTemplate(); final String output = templatingService.renderTemplate(detection, template); assertThat(output) - .contains("name-4") + .contains("name_4") + .contains("value_C") + .doesNotContain("value_C2") // The dup one .contains(detection.getDetectorName()); } @@ -57,15 +58,41 @@ void renderDefaultTemplate() { void renderDefaultTemplate_noValues_noLinkedEvents() { final RuleEmailTemplatingService templatingService = new RuleEmailTemplatingService(); final Detection detection = getExampleDetection(false, false); - @SuppressWarnings("checkstyle:LineLength") final String template = new AnalyticUiDefaultConfig() - .getDefaultBodyTemplate(); + final String template = new AnalyticUiDefaultConfig().getDefaultBodyTemplate(); final String output = templatingService.renderTemplate(detection, template); assertThat(output) - .doesNotContain("name-4") + .doesNotContain("name_4") .contains(detection.getDetectorName()); } + @Test + void variableNames() { + final RuleEmailTemplatingService templatingService = new RuleEmailTemplatingService(); + final String template = """ + {{ values['key-1'] }} + {{ values['key_2'] }} + {{ values['key/3'] }} + {{ values['key.4'] }} + """; + + final Detection detection = Detection.builder() + .withDetectorName("MyName") + .addValue("key-1", "val_1") + .addValue("key_2", "val_2") + .addValue("key/3", "val_3") + .addValue("key.4", "val_4") + .build(); + + final String output = templatingService.renderTemplate(detection, template); + + assertThat(output) + .contains("val_1") + .contains("val_2") + .contains("val_3") + .contains("val_4"); + } + private Detection getExampleDetection(final boolean includeValues, final boolean includeLinkedEvents) { final String stroom = "Test Environment"; @@ -90,10 +117,11 @@ private Detection getExampleDetection(final boolean includeValues, if (includeValues) { builder - .addValue("name-1", "value-A") - .addValue("name-2", "value-B") - .addValue("name-3", "value-C") - .addValue("name-4", null); + .addValue("name_1", "value_A") + .addValue("name_2", "value_B") + .addValue("name_3", "value_C") + .addValue("name_4", null) + .addValue("name_3", "value_C2"); // Dup } if (includeLinkedEvents) { builder diff --git a/unreleased_changes/20240919_153021_934__0.md b/unreleased_changes/20240919_153021_934__0.md new file mode 100644 index 00000000000..71230f27e66 --- /dev/null +++ b/unreleased_changes/20240919_153021_934__0.md @@ -0,0 +1,19 @@ +* Change the key names in the example rule detection to remove `-`. Not sensible to encourage keys with a `-` in them as that prevents doing `values.key-1`. Also add a warning if there are multiple detection values with the same name/key (only the first will be used in each case). + + +```sh +# ONLY the top line will be included as a change entry in the CHANGELOG. +# The entry should be in GitHub flavour markdown and should be written on a SINGLE +# line with no hard breaks. You can have multiple change files for a single GitHub issue. +# The entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than +# 'Fixed nasty bug'. +# +# Examples of acceptable entries are: +# +# +# * Issue **123** : Fix bug with an associated GitHub issue in this repository +# +# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository +# +# * Fix bug with no associated GitHub issue. +```