Merge remote-tracking branch 'remotes/origin/#219-duplicate-attachment-names'

Author: bbottema

src/main/java/org/simplejavamail/converter/EmailConverter.java

@@ -283,7 +283,7 @@ private static void buildEmailFromMimeMessage(@Nonnull final EmailPopulatingBuil
 			final String cidName = checkNonEmptyArgument(cid.getKey(), "cid.key");
 			builder.withEmbeddedImage(extractCID(cidName), cid.getValue());
 		}
-		for (final Map.Entry<String, DataSource> attachment : parsed.getAttachmentList().entrySet()) {
+		for (final Map.Entry<String, DataSource> attachment : parsed.getAttachmentList()) {
 			builder.withAttachment(extractCID(attachment.getKey()), attachment.getValue());
 		}
 	}

src/main/java/org/simplejavamail/converter/internal/mimemessage/MimeMessageParser.java

@@ -1,5 +1,7 @@
 package org.simplejavamail.converter.internal.mimemessage;
 
+import org.simplejavamail.internal.util.NaturalEntryKeyComparator;
+
 import javax.activation.DataHandler;
 import javax.activation.DataSource;
 import javax.annotation.Nonnull;
@@ -25,14 +27,8 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.TreeMap;
+import java.util.*;
+import java.util.AbstractMap.SimpleEntry;
 
 import static java.lang.String.format;
 import static org.simplejavamail.internal.util.MiscUtil.extractCID;
@@ -133,13 +129,13 @@ private static void parseMimePartTree(@Nonnull final MimePart currentPart, @Nonn
 			final DataSource ds = createDataSource(currentPart);
 			// if the diposition is not provided, for now the part should be treated as inline (later non-embedded inline attachments are moved)
 			if (Part.ATTACHMENT.equalsIgnoreCase(disposition)) {
-				parsedComponents.attachmentList.put(parseResourceName(parseContentID(currentPart), parseFileName(currentPart)), ds);
+				parsedComponents.attachmentList.add(new SimpleEntry<>(parseResourceNameOrUnnamed(parseContentID(currentPart), parseFileName(currentPart)), ds));
 			} else if (disposition == null || Part.INLINE.equalsIgnoreCase(disposition)) {
 				if (parseContentID(currentPart) != null) {
 					parsedComponents.cidMap.put(parseContentID(currentPart), ds);
 				} else {
 					// contentID missing -> treat as standard attachment
-					parsedComponents.attachmentList.put(parseResourceName(null, parseFileName(currentPart)), ds);
+					parsedComponents.attachmentList.add(new SimpleEntry<>(parseResourceNameOrUnnamed(null, parseFileName(currentPart)), ds));
 				}
 			} else {
 				throw new IllegalStateException("invalid attachment type");
@@ -219,7 +215,13 @@ public static String parseDisposition(@Nonnull final MimePart currentPart) {
 	}
 
 	@Nonnull
-	private static String parseResourceName(@Nullable final String possibleWrappedContentID, @Nonnull final String fileName) {
+	private static String parseResourceNameOrUnnamed(@Nullable final String possibleWrappedContentID, @Nonnull final String fileName) {
+		String resourceName = parseResourceName(possibleWrappedContentID, fileName);
+		return valueNullOrEmpty(resourceName) ? "unnamed" : resourceName;
+	}
+	
+	@Nonnull
+	private static String parseResourceName(@Nullable String possibleWrappedContentID, @Nonnull String fileName) {
 		if (!valueNullOrEmpty(possibleWrappedContentID)) {
 			// https://regex101.com/r/46ulb2/1
 			String unwrappedContentID = possibleWrappedContentID.replaceAll("^<?(.*?)>?$", "$1");
@@ -231,7 +233,7 @@ private static String parseResourceName(@Nullable final String possibleWrappedCo
 			return fileName;
 		}
 	}
-
+	
 	@SuppressWarnings("WeakerAccess")
 	@Nonnull
 	public static List<Header> retrieveAllHeaders(@Nonnull final MimePart part) {
@@ -454,14 +456,15 @@ static void moveInvalidEmbeddedResourcesToAttachments(ParsedMimeMessageComponent
 			Map.Entry<String, DataSource> cidEntry = it.next();
 			String cid = extractCID(cidEntry.getKey());
 			if (htmlContent == null || !htmlContent.contains("cid:" + cid)) {
-				parsedComponents.attachmentList.put(cid, cidEntry.getValue());
+				parsedComponents.attachmentList.add(new SimpleEntry<>(cid, cidEntry.getValue()));
 				it.remove();
 			}
 		}
 	}
 
 	public static class ParsedMimeMessageComponents {
-		final Map<String, DataSource> attachmentList = new TreeMap<>();
+		@SuppressWarnings("unchecked")
+		final Set<Map.Entry<String, DataSource>> attachmentList = new TreeSet<>(NaturalEntryKeyComparator.INSTANCE);
 		final Map<String, DataSource> cidMap = new TreeMap<>();
 		private final Map<String, Object> headers = new HashMap<>();
 		private final List<InternetAddress> toAddresses = new ArrayList<>();
@@ -481,7 +484,7 @@ public String getMessageId() {
 			return messageId;
 		}
 
-		public Map<String, DataSource> getAttachmentList() {
+		public Set<Map.Entry<String, DataSource>> getAttachmentList() {
 			return attachmentList;
 		}
 

src/main/java/org/simplejavamail/internal/util/NaturalEntryKeyComparator.java

@@ -0,0 +1,19 @@
+package org.simplejavamail.internal.util;
+
+import java.util.Comparator;
+import java.util.Map;
+
+public class NaturalEntryKeyComparator<T extends Comparable<T>> implements Comparator<Map.Entry<T, Object>> {
+	
+	public static final NaturalEntryKeyComparator INSTANCE = new NaturalEntryKeyComparator();
+	
+	// TODO Lombok
+	private NaturalEntryKeyComparator(){
+	}
+	
+	
+	@Override
+	public int compare(Map.Entry<T, Object> o1, Map.Entry<T, Object> o2) {
+		return o1.getKey().compareTo(o2.getKey());
+	}
+}

src/test/java/org/simplejavamail/converter/internal/mimemessage/MimeMessageParserTest.java

@@ -18,8 +18,9 @@ public void testMoveInvalidEmbeddedResourcesToAttachments_NoHtmlNoInvalid() thro
 		moveInvalidEmbeddedResourcesToAttachments(parsedComponents);
 
 		assertThat(parsedComponents.cidMap).isEmpty();
-		assertThat(parsedComponents.attachmentList).containsOnlyKeys("moo1","moo2");
+		assertThat(parsedComponents.attachmentList).extracting("key").containsOnly("moo1", "moo2");
 	}
+	
 	@Test
 	public void testMoveInvalidEmbeddedResourcesToAttachments_HtmlButNoInvalid() throws IOException {
 		ParsedMimeMessageComponents parsedComponents = new ParsedMimeMessageComponents();
@@ -29,7 +30,7 @@ public void testMoveInvalidEmbeddedResourcesToAttachments_HtmlButNoInvalid() thr
 		moveInvalidEmbeddedResourcesToAttachments(parsedComponents);
 
 		assertThat(parsedComponents.cidMap).isEmpty();
-		assertThat(parsedComponents.attachmentList).containsOnlyKeys("moo1","moo2");
+		assertThat(parsedComponents.attachmentList).extracting("key").containsOnly("moo1", "moo2");
 	}
 
 	@Test
@@ -41,6 +42,6 @@ public void testMoveInvalidEmbeddedResourcesToAttachments_Invalid() throws IOExc
 		moveInvalidEmbeddedResourcesToAttachments(parsedComponents);
 
 		assertThat(parsedComponents.cidMap).containsOnlyKeys("moo1");
-		assertThat(parsedComponents.attachmentList).containsOnlyKeys("moo2");
+		assertThat(parsedComponents.attachmentList).extracting("key").containsOnly("moo2");
 	}
 }