diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/email/ExecutableEmailAction.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/email/ExecutableEmailAction.java index 1f8e87cad1f..c9a8bfca0ad 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/email/ExecutableEmailAction.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/email/ExecutableEmailAction.java @@ -25,9 +25,12 @@ import org.elasticsearch.xpack.watcher.support.Variables; import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; public class ExecutableEmailAction extends ExecutableAction { + private static final AtomicLong counter = new AtomicLong(0); + private final EmailService emailService; private final TextTemplateEngine templateEngine; private final HtmlSanitizer htmlSanitizer; @@ -67,7 +70,10 @@ public class ExecutableEmailAction extends ExecutableAction { } Email.Builder email = action.getEmail().render(templateEngine, model, htmlSanitizer, attachments); - email.id(actionId + "_" + ctx.id().value()); + // the counter ensures that a different message id is generated, even if the method is called with the same parameters + // this may happen if a foreach loop is used for this action + // same message ids will result in emails not being accepted by mail servers and thus have to be prevented at all times + email.id(actionId + "_" + ctx.id().value() + "_" + Long.toUnsignedString(counter.incrementAndGet())); if (ctx.simulateAction(actionId)) { return new EmailAction.Result.Simulated(email.build()); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java index 037a70602ba..9ed94359689 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java @@ -69,6 +69,7 @@ import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; @@ -170,7 +171,7 @@ public class EmailActionTests extends ESTestCase { 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("_id_" + wid.value())); + assertThat(actualEmail.id(), startsWith("_id_" + wid.value() + "_")); assertThat(actualEmail, notNullValue()); assertThat(actualEmail.subject(), is(subject == null ? null : subject.getTemplate())); assertThat(actualEmail.textBody(), is(textBody == null ? null : textBody.getTemplate())); @@ -178,6 +179,12 @@ public class EmailActionTests extends ESTestCase { if (dataAttachment != null) { assertThat(actualEmail.attachments(), hasKey("data")); } + + // a second execution with the same parameters may not yield the same message id + result = executable.execute("_id", ctx, payload); + String oldMessageId = actualEmail.id(); + String newMessageId = ((EmailAction.Result.Success) result).email().id(); + assertThat(oldMessageId, is(not(newMessageId))); } public void testParser() throws Exception {