Watcher: Ensure mail message ids are unique per watch action

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/email/ExecutableEmailAction.java

@@ -63,7 +63,7 @@ public Action.Result execute(String actionId, WatchExecutionContext ctx, Payload
         }
 
         Email.Builder email = action.getEmail().render(templateEngine, model, htmlSanitizer, attachments);
-        email.id(ctx.id().value());
+        email.id(actionId + "_" + ctx.id().value());
 
         if (ctx.simulateAction(actionId)) {
             return new EmailAction.Result.Simulated(email.build());

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Email.java

@@ -354,7 +354,7 @@ public Builder attach(Attachment attachment) {
          * after this is called is incorrect.
          */
         public Email build() {
-            assert id != null : "email id should not be null (should be set to the watch id";
+            assert id != null : "email id should not be null";
             Email email = new Email(id, from, replyTo, priority, sentDate, to, cc, bcc, subject, textBody, htmlBody,
                     unmodifiableMap(attachments));
             attachments = null;

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java

@@ -171,7 +171,7 @@ public void testExecute() throws Exception {
         assertThat(result, instanceOf(EmailAction.Result.Success.class));
         assertThat(((EmailAction.Result.Success) result).account(), equalTo(account));
         Email actualEmail = ((EmailAction.Result.Success) result).email();
-        assertThat(actualEmail.id(), is(wid.value()));
+        assertThat(actualEmail.id(), is("_id_" + wid.value()));
         assertThat(actualEmail, notNullValue());
         assertThat(actualEmail.subject(), is(subject == null ? null : subject.getTemplate()));
         assertThat(actualEmail.textBody(), is(textBody == null ? null : textBody.getTemplate()));

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailMessageIdTests.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.watcher.actions.email;
+
+import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext;
+import org.elasticsearch.xpack.core.watcher.watch.Payload;
+import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine;
+import org.elasticsearch.xpack.watcher.notification.email.EmailService;
+import org.elasticsearch.xpack.watcher.notification.email.EmailTemplate;
+import org.elasticsearch.xpack.watcher.notification.email.HtmlSanitizer;
+import org.elasticsearch.xpack.watcher.notification.email.support.EmailServer;
+import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine;
+import org.elasticsearch.xpack.watcher.test.WatcherTestUtils;
+import org.junit.After;
+import org.junit.Before;
+
+import javax.mail.internet.MimeMessage;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.hamcrest.Matchers.hasSize;
+
+public class EmailMessageIdTests extends ESTestCase {
+
+    private EmailServer server;
+    private TextTemplateEngine textTemplateEngine = new MockTextTemplateEngine();
+    private HtmlSanitizer htmlSanitizer = new HtmlSanitizer(Settings.EMPTY);
+    private EmailService emailService;
+    private EmailAction emailAction;
+
+    @Before
+    public void startSmtpServer() {
+        server = EmailServer.localhost(logger);
+
+        Settings settings = Settings.builder()
+                .put("xpack.notification.email.account.test.smtp.auth", true)
+                .put("xpack.notification.email.account.test.smtp.user", EmailServer.USERNAME)
+                .put("xpack.notification.email.account.test.smtp.password", EmailServer.PASSWORD)
+                .put("xpack.notification.email.account.test.smtp.port", server.port())
+                .put("xpack.notification.email.account.test.smtp.host", "localhost")
+                .build();
+
+        Set<Setting<?>> registeredSettings = new HashSet<>(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
+        registeredSettings.addAll(EmailService.getSettings());
+        ClusterSettings clusterSettings = new ClusterSettings(settings, registeredSettings);
+        emailService = new EmailService(settings, null, clusterSettings);
+        EmailTemplate emailTemplate = EmailTemplate.builder().from("[email protected]").to("[email protected]")
+                .subject("subject").textBody("body").build();
+        emailAction = new EmailAction(emailTemplate, null, null, null, null, null);
+    }
+
+    @After
+    public void stopSmtpServer() {
+        server.stop();
+    }
+
+    public void testThatMessageIdIsUnique() throws Exception {
+        List<MimeMessage> messages = new ArrayList<>();
+        server.addListener(messages::add);
+        ExecutableEmailAction firstEmailAction = new ExecutableEmailAction(emailAction, logger, emailService, textTemplateEngine,
+                htmlSanitizer, Collections.emptyMap());
+        ExecutableEmailAction secondEmailAction = new ExecutableEmailAction(emailAction, logger, emailService, textTemplateEngine,
+                htmlSanitizer, Collections.emptyMap());
+
+        WatchExecutionContext ctx = WatcherTestUtils.createWatchExecutionContext(logger);
+        firstEmailAction.execute("my_first_action_id", ctx, Payload.EMPTY);
+        secondEmailAction.execute("my_second_action_id", ctx, Payload.EMPTY);
+
+        assertThat(messages, hasSize(2));
+        // check for unique message ids, should be two as well
+        Set<String> messageIds = new HashSet<>();
+        for (MimeMessage message : messages) {
+            messageIds.add(message.getMessageID());
+        }
+        assertThat(messageIds, hasSize(2));
+    }
+}
+