diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/DataAttachment.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/DataAttachment.java index 205c4d80d4f..a5480eb4ee7 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/DataAttachment.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/DataAttachment.java @@ -34,7 +34,7 @@ public enum DataAttachment implements ToXContent { @Override public Attachment create(String id, Map data) { - return new Attachment.XContent.Yaml(id, "data.yml", new Payload.Simple(data)); + return new Attachment.XContent.Yaml(id, id, new Payload.Simple(data)); } @Override @@ -51,7 +51,7 @@ public enum DataAttachment implements ToXContent { @Override public Attachment create(String id, Map data) { - return new Attachment.XContent.Json(id, "data.json", new Payload.Simple(data)); + return new Attachment.XContent.Json(id, id, new Payload.Simple(data)); } @Override diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachment.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachment.java index 618aa12a88e..258723f3415 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachment.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachment.java @@ -32,7 +32,6 @@ public class DataAttachment implements EmailAttachmentParser.EmailAttachment { } else { builder.field("format", "json"); } - return builder.endObject().endObject(); } diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParser.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParser.java index ebde963ab9c..f8d864975d2 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParser.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParser.java @@ -25,6 +25,11 @@ public interface EmailAttachmentParser attachments; + private final Collection attachments; - public EmailAttachments(List attachments) { + public EmailAttachments(Collection attachments) { this.attachments = attachments; } - public List getAttachments() { + public Collection getAttachments() { return attachments; } diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentsParser.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentsParser.java index 91834d239fe..fc795298072 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentsParser.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentsParser.java @@ -12,7 +12,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.ArrayList; -import java.util.List; +import java.util.LinkedHashMap; import java.util.Map; public class EmailAttachmentsParser { @@ -25,7 +25,7 @@ public class EmailAttachmentsParser { } public EmailAttachments parse(XContentParser parser) throws IOException { - List attachments = new ArrayList<>(); + Map attachments = new LinkedHashMap<>(); String currentFieldName = null; XContentParser.Token token; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -41,17 +41,21 @@ public class EmailAttachmentsParser { EmailAttachmentParser emailAttachmentParser = parsers.get(currentAttachmentType); if (emailAttachmentParser == null) { - throw new ElasticsearchParseException("Cannot parse attachment of type " + currentAttachmentType); + throw new ElasticsearchParseException("Cannot parse attachment of type [{}]", currentAttachmentType); } EmailAttachmentParser.EmailAttachment emailAttachment = emailAttachmentParser.parse(currentFieldName, parser); - attachments.add(emailAttachment); + if (attachments.containsKey(emailAttachment.id())) { + throw new ElasticsearchParseException("Attachment with id [{}] has already been created, must be renamed", + emailAttachment.id()); + } + attachments.put(emailAttachment.id(), emailAttachment); // one further to skip the end_object from the attachment parser.nextToken(); } } } - return new EmailAttachments(attachments); + return new EmailAttachments(new ArrayList<>(attachments.values())); } public Map getParsers() { diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParser.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParser.java index 0c3fa669adf..0516015ec99 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParser.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParser.java @@ -94,7 +94,7 @@ public class HttpEmailAttachementParser implements EmailAttachmentParser data = singletonMap("key", "value"); Attachment attachment = DataAttachment.JSON.create("data", data); diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/EmailActionTests.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/EmailActionTests.java index 984c443c2e3..ed4d9867233 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/EmailActionTests.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/EmailActionTests.java @@ -481,7 +481,7 @@ public class EmailActionTests extends ESTestCase { } public void testThatDataAttachmentGetsAttachedWithId() throws Exception { - String attachmentId = "my_attachment"; + String attachmentId = randomAsciiOfLength(10) + ".yml"; XContentBuilder builder = jsonBuilder().startObject() .startObject("attachments") @@ -506,9 +506,9 @@ public class EmailActionTests extends ESTestCase { EmailAction.Result.Success successResult = (EmailAction.Result.Success) result; Map attachments = successResult.email().attachments(); - assertThat(attachments, hasKey("my_attachment")); - Attachment dataAttachment = attachments.get("my_attachment"); - assertThat(dataAttachment.name(), is("data.yml")); + assertThat(attachments, hasKey(attachmentId)); + Attachment dataAttachment = attachments.get(attachmentId); + assertThat(dataAttachment.name(), is(attachmentId)); assertThat(dataAttachment.type(), is("yaml")); assertThat(dataAttachment.contentType(), is("application/yaml")); } diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachmentParserTests.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachmentParserTests.java index 846a1d6b04e..7cf3ecd5e80 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachmentParserTests.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/DataAttachmentParserTests.java @@ -11,7 +11,9 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ESTestCase; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -36,7 +38,8 @@ public class DataAttachmentParserTests extends ESTestCase { assertThat(emailAttachments.getAttachments(), hasSize(1)); XContentBuilder toXcontentBuilder = jsonBuilder().startObject(); - emailAttachments.getAttachments().get(0).toXContent(toXcontentBuilder, ToXContent.EMPTY_PARAMS); + List attachments = new ArrayList<>(emailAttachments.getAttachments()); + attachments.get(0).toXContent(toXcontentBuilder, ToXContent.EMPTY_PARAMS); toXcontentBuilder.endObject(); assertThat(toXcontentBuilder.string(), is(builder.string())); } diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParsersTests.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParsersTests.java index 7566f161099..040346d5ccd 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParsersTests.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/EmailAttachmentParsersTests.java @@ -61,15 +61,15 @@ public class EmailAttachmentParsersTests extends ESTestCase { EmailAttachments attachments = parser.parse(xContentParser); assertThat(attachments.getAttachments(), hasSize(2)); - EmailAttachmentParser.EmailAttachment emailAttachment = attachments.getAttachments().get(0); + List emailAttachments = new ArrayList<>(attachments.getAttachments()); + EmailAttachmentParser.EmailAttachment emailAttachment = emailAttachments.get(0); assertThat(emailAttachment, instanceOf(TestEmailAttachment.class)); Attachment attachment = parsers.get("test").toAttachment(ctx, new Payload.Simple(), emailAttachment); assertThat(attachment.name(), is("my-id")); assertThat(attachment.contentType(), is("personalContentType")); - assertThat(parsers.get("test").toAttachment(ctx, new Payload.Simple(), - attachments.getAttachments().get(1)).id(), is("my-other-id")); + assertThat(parsers.get("test").toAttachment(ctx, new Payload.Simple(), emailAttachments.get(1)).id(), is("my-other-id")); } public void testThatUnknownParserThrowsException() throws IOException { @@ -84,13 +84,13 @@ public class EmailAttachmentParsersTests extends ESTestCase { parser.parse(xContentParser); fail("Expected random parser of type [" + type + "] to throw an exception"); } catch (ElasticsearchParseException e) { - assertThat(e.getMessage(), containsString("Cannot parse attachment of type " + type)); + assertThat(e.getMessage(), containsString("Cannot parse attachment of type [" + type + "]")); } } public void testThatToXContentSerializationWorks() throws Exception { List attachments = new ArrayList<>(); - attachments.add(new DataAttachment("my-id", org.elasticsearch.watcher.actions.email.DataAttachment.JSON)); + attachments.add(new DataAttachment("my-name.json", org.elasticsearch.watcher.actions.email.DataAttachment.JSON)); HttpRequestTemplate requestTemplate = HttpRequestTemplate.builder("localhost", 80).scheme(Scheme.HTTP).path("/").build(); HttpRequestAttachment httpRequestAttachment = new HttpRequestAttachment("other-id", requestTemplate, null); @@ -100,13 +100,36 @@ public class EmailAttachmentParsersTests extends ESTestCase { XContentBuilder builder = jsonBuilder(); emailAttachments.toXContent(builder, ToXContent.EMPTY_PARAMS); logger.info("JSON is: " + builder.string()); - assertThat(builder.string(), containsString("my-id")); + assertThat(builder.string(), containsString("my-name.json")); assertThat(builder.string(), containsString("json")); assertThat(builder.string(), containsString("other-id")); assertThat(builder.string(), containsString("localhost")); assertThat(builder.string(), containsString("/")); } + public void testThatTwoAttachmentsWithTheSameIdThrowError() throws Exception { + Map parsers = new HashMap<>(); + parsers.put("test", new TestEmailAttachmentParser()); + EmailAttachmentsParser parser = new EmailAttachmentsParser(parsers); + + List attachments = new ArrayList<>(); + attachments.add(new TestEmailAttachment("my-name.json", "value")); + attachments.add(new TestEmailAttachment("my-name.json", "value")); + + EmailAttachments emailAttachments = new EmailAttachments(attachments); + XContentBuilder builder = jsonBuilder(); + emailAttachments.toXContent(builder, ToXContent.EMPTY_PARAMS); + logger.info("JSON is: " + builder.string()); + + XContentParser xContentParser = JsonXContent.jsonXContent.createParser(builder.bytes()); + try { + parser.parse(xContentParser); + fail("Expected parser to fail but did not happen"); + } catch (ElasticsearchParseException e) { + assertThat(e.getMessage(), is("Attachment with id [my-name.json] has already been created, must be renamed")); + } + } + public class TestEmailAttachmentParser implements EmailAttachmentParser { @Override @@ -138,7 +161,7 @@ public class EmailAttachmentParsersTests extends ESTestCase { @Override public Attachment toAttachment(WatchExecutionContext ctx, Payload payload, TestEmailAttachment attachment) { - return new Attachment.Bytes(attachment.getId(), attachment.getValue().getBytes(Charsets.UTF_8), "personalContentType"); + return new Attachment.Bytes(attachment.id(), attachment.getValue().getBytes(Charsets.UTF_8), "personalContentType"); } } @@ -165,7 +188,8 @@ public class EmailAttachmentParsersTests extends ESTestCase { return value; } - public String getId() { + @Override + public String id() { return id; } diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParserTests.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParserTests.java index fd5ea57b9d1..1d41d3e3060 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParserTests.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/watcher/actions/email/service/attachment/HttpEmailAttachementParserTests.java @@ -21,7 +21,9 @@ import org.elasticsearch.watcher.support.secret.SecretService; import org.elasticsearch.watcher.test.MockTextTemplateEngine; import org.junit.Before; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import static java.nio.charset.StandardCharsets.UTF_8; @@ -83,7 +85,8 @@ public class HttpEmailAttachementParserTests extends ESTestCase { assertThat(emailAttachments.getAttachments(), hasSize(1)); XContentBuilder toXcontentBuilder = jsonBuilder().startObject(); - emailAttachments.getAttachments().get(0).toXContent(toXcontentBuilder, ToXContent.EMPTY_PARAMS); + List attachments = new ArrayList<>(emailAttachments.getAttachments()); + attachments.get(0).toXContent(toXcontentBuilder, ToXContent.EMPTY_PARAMS); toXcontentBuilder.endObject(); assertThat(toXcontentBuilder.string(), is(builder.string())); }