#219, #249, #310, #351: Properly fix support for duplicate file names this time

Author: bbottema

modules/core-module/src/main/java/org/simplejavamail/internal/util/NaturalEntryKeyComparator.java

@@ -0,0 +1,23 @@
+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) {
+		int keyComparison = o1.getKey().compareTo(o2.getKey());
+		if (keyComparison != 0) {
+			return keyComparison;
+		}
+		return Integer.compare(o1.getValue().hashCode(), o2.getValue().hashCode());
+	}
+}

modules/simple-java-mail/src/main/java/org/simplejavamail/converter/EmailConverter.java

@@ -682,7 +682,7 @@ private static EmailPopulatingBuilder buildEmailFromMimeMessage(@NotNull final E
 			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());
 		}
 		return builder;

modules/simple-java-mail/src/main/java/org/simplejavamail/converter/internal/mimemessage/MimeMessageParser.java

@@ -4,6 +4,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.simplejavamail.internal.util.MiscUtil;
+import org.simplejavamail.internal.util.NaturalEntryKeyComparator;
 import org.simplejavamail.internal.util.Preconditions;
 
 import javax.activation.ActivationDataFlavor;
@@ -29,6 +30,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
+import java.util.AbstractMap.SimpleEntry;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -38,7 +40,9 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.TreeSet;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -160,13 +164,13 @@ private static void parseMimePartTree(@NotNull final MimePart currentPart, @NotN
 			final DataSource ds = createDataSource(currentPart, fetchAttachmentData);
 			// 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(parseResourceNameOrUnnamed(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(parseResourceNameOrUnnamed(null, parseFileName(currentPart)), ds);
+					parsedComponents.attachmentList.add(new SimpleEntry<>(parseResourceNameOrUnnamed(null, parseFileName(currentPart)), ds));
 				}
 			} else {
 				throw new IllegalStateException("invalid attachment type");
@@ -549,14 +553,15 @@ static void moveInvalidEmbeddedResourcesToAttachments(ParsedMimeMessageComponent
 			Map.Entry<String, DataSource> cidEntry = it.next();
 			String cid = extractCID(cidEntry.getKey());
 			if (!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, Collection<Object>> headers = new HashMap<>();
 		private final List<InternetAddress> toAddresses = new ArrayList<>();
@@ -580,7 +585,7 @@ public String getMessageId() {
 			return messageId;
 		}
 
-		public Map<String, DataSource> getAttachmentList() {
+		public Set<Map.Entry<String, DataSource>> getAttachmentList() {
 			return attachmentList;
 		}
 

modules/simple-java-mail/src/test/java/org/simplejavamail/converter/internal/mimemessage/MimeMessageParserTest.java

@@ -66,7 +66,7 @@ public void testBasicParsing()
 		assertThat(mimeMessageParts.getSentDate()).isBetween(receiveWindowStart.getTime(), new Date());
 
 		assertThat(mimeMessageParts.getCidMap()).containsOnlyKeys("<thumbsup>");
-		assertThat(mimeMessageParts.getAttachmentList()).containsOnlyKeys("dresscode.txt", "location.txt");
+		assertThat(mimeMessageParts.getAttachmentList()).extracting("key").containsOnly("dresscode.txt", "location.txt");
 	}
 
 	@Test
@@ -78,7 +78,7 @@ public void testAttachmentNameResolution()
 		assertThat(components.getHtmlContent()).isNull();
 		assertThat(components.getPlainContent()).isEqualTo("body text");
 		assertThat(components.getCidMap()).isEmpty();
-		assertThat(components.getAttachmentList()).containsOnlyKeys("proper-name.txt");
+		assertThat(components.getAttachmentList()).extracting("key").containsOnly("proper-name.txt");
 	}
 
 	private MimeMessage produceMimeMessageWithNamingIssue()
@@ -126,7 +126,7 @@ public void testMoveInvalidEmbeddedResourcesToAttachments_NoHtmlNoInvalid() thro
 		moveInvalidEmbeddedResourcesToAttachments(parsedComponents);
 
 		assertThat(parsedComponents.cidMap).isEmpty();
-		assertThat(parsedComponents.attachmentList).containsKeys("moo1", "moo2");
+		assertThat(parsedComponents.attachmentList).extracting("key").containsOnly("moo1", "moo2");
 	}
 
 	@Test
@@ -138,7 +138,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
@@ -150,7 +150,7 @@ 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");
 	}
 
 	@Test