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.
gchq · Sep 19, 2024 · 68f4cf0 · 68f4cf0
1 parent d9d0a1b
commit 68f4cf0
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 24 deletions.
diff --git a/...ytics/stroom-analytics-impl/src/main/java/stroom/analytics/impl/AnalyticsServiceImpl.java b/...ytics/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-impl/src/main/java/stroom/analytics/impl/RuleEmailTemplatingService.java b/...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<String, Object> 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<String, String> 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<String, String> map = new HashMap<>(values.size());
+                final Set<String> 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/...s/stroom-analytics-impl/src/test/java/stroom/analytics/impl/TestAnalyticsServiceImpl.java b/...s/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("<li><strong>name_1</strong>: value_A</li>")
+                .contains("<li><strong>name_4</strong>: </li>")
+                .contains("<li>Environment: Test Environment, Stream ID: 1001, Event ID: 2</li>");
+    }
+}
diff --git a/...om-analytics-impl/src/test/java/stroom/analytics/impl/TestRuleEmailTemplatingService.java b/...om-analytics-impl/src/test/java/stroom/analytics/impl/TestRuleEmailTemplatingService.java
@@ -43,29 +43,56 @@ 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());
     }
 
     @Test
     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
@@ -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.
+```