JavaMailSenderImpl catches Exception instead of MessagingException and checks for reconnect in case of message batches

Issue: SPR-12298

Author: jhoeller

spring-context-support/src/main/java/org/springframework/mail/javamail/JavaMailSenderImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -330,7 +330,7 @@ public MimeMessage createMimeMessage(InputStream contentStream) throws MailExcep
 		try {
 			return new MimeMessage(getSession(), contentStream);
 		}
-		catch (MessagingException ex) {
+		catch (Exception ex) {
 			throw new MailParseException("Could not parse raw MIME content", ex);
 		}
 	}
@@ -385,35 +385,40 @@ public void send(MimeMessagePreparator... mimeMessagePreparators) throws MailExc
 	 * in case of failure when sending a message
 	 */
 	protected void doSend(MimeMessage[] mimeMessages, Object[] originalMessages) throws MailException {
-		String username = getUsername();
-		String password = getPassword();
-		if ("".equals(username)) {  // probably from a placeholder
-			username = null;
-			if ("".equals(password)) {  // in conjunction with "" username, this means no password to use
-				password = null;
-			}
-		}
-
 		Map<Object, Exception> failedMessages = new LinkedHashMap<Object, Exception>();
-		Transport transport;
-		try {
-			transport = getTransport(getSession());
-			transport.connect(getHost(), getPort(), username, password);
-		}
-		catch (AuthenticationFailedException ex) {
-			throw new MailAuthenticationException(ex);
-		}
-		catch (MessagingException ex) {
-			// Effectively, all messages failed...
-			for (int i = 0; i < mimeMessages.length; i++) {
-				Object original = (originalMessages != null ? originalMessages[i] : mimeMessages[i]);
-				failedMessages.put(original, ex);
-			}
-			throw new MailSendException("Mail server connection failed", ex, failedMessages);
-		}
+		Transport transport = null;
 
 		try {
 			for (int i = 0; i < mimeMessages.length; i++) {
+
+				// Check transport connection first...
+				if (transport == null || !transport.isConnected()) {
+					if (transport != null) {
+						try {
+							transport.close();
+						}
+						catch (Exception ex) {
+							// Ignore - we're reconnecting anyway
+						}
+						transport = null;
+					}
+					try {
+						transport = connectTransport();
+					}
+					catch (AuthenticationFailedException ex) {
+						throw new MailAuthenticationException(ex);
+					}
+					catch (Exception ex) {
+						// Effectively, all remaining messages failed...
+						for (int j = i; j < mimeMessages.length; j++) {
+							Object original = (originalMessages != null ? originalMessages[j] : mimeMessages[j]);
+							failedMessages.put(original, ex);
+						}
+						throw new MailSendException("Mail server connection failed", ex, failedMessages);
+					}
+				}
+
+				// Send message via current transport...
 				MimeMessage mimeMessage = mimeMessages[i];
 				try {
 					if (mimeMessage.getSentDate() == null) {
@@ -427,17 +432,19 @@ protected void doSend(MimeMessage[] mimeMessages, Object[] originalMessages) thr
 					}
 					transport.sendMessage(mimeMessage, mimeMessage.getAllRecipients());
 				}
-				catch (MessagingException ex) {
+				catch (Exception ex) {
 					Object original = (originalMessages != null ? originalMessages[i] : mimeMessage);
 					failedMessages.put(original, ex);
 				}
 			}
 		}
 		finally {
 			try {
-				transport.close();
+				if (transport != null) {
+					transport.close();
+				}
 			}
-			catch (MessagingException ex) {
+			catch (Exception ex) {
 				if (!failedMessages.isEmpty()) {
 					throw new MailSendException("Failed to close server connection after message failures", ex,
 							failedMessages);
@@ -453,11 +460,39 @@ protected void doSend(MimeMessage[] mimeMessages, Object[] originalMessages) thr
 		}
 	}
 
+	/**
+	 * Obtain and connect a Transport from the underlying JavaMail Session,
+	 * passing in the specified host, port, username, and password.
+	 * @return the connected Transport object
+	 * @throws MessagingException if the connect attempt failed
+	 * @since 4.1.2
+	 * @see #getTransport
+	 * @see #getHost()
+	 * @see #getPort()
+	 * @see #getUsername()
+	 * @see #getPassword()
+	 */
+	protected Transport connectTransport() throws MessagingException {
+		String username = getUsername();
+		String password = getPassword();
+		if ("".equals(username)) {  // probably from a placeholder
+			username = null;
+			if ("".equals(password)) {  // in conjunction with "" username, this means no password to use
+				password = null;
+			}
+		}
+
+		Transport transport = getTransport(getSession());
+		transport.connect(getHost(), getPort(), username, password);
+		return transport;
+	}
+
 	/**
 	 * Obtain a Transport object from the given JavaMail Session,
 	 * using the configured protocol.
 	 * <p>Can be overridden in subclasses, e.g. to return a mock Transport object.
 	 * @see javax.mail.Session#getTransport(String)
+	 * @see #getSession()
 	 * @see #getProtocol()
 	 */
 	protected Transport getTransport(Session session) throws NoSuchProviderException {

spring-context-support/src/test/java/org/springframework/mail/javamail/JavaMailSenderTests.java

@@ -23,7 +23,6 @@
 import java.util.GregorianCalendar;
 import java.util.List;
 import java.util.Properties;
-
 import javax.activation.FileTypeMap;
 import javax.mail.Address;
 import javax.mail.Message;
@@ -41,6 +40,7 @@
 import org.springframework.mail.MailParseException;
 import org.springframework.mail.MailSendException;
 import org.springframework.mail.SimpleMailMessage;
+import org.springframework.util.ObjectUtils;
 
 /**
  * @author Juergen Hoeller
@@ -106,7 +106,7 @@ public void testJavaMailSenderWithSimpleMessages() throws MessagingException, IO
 		simpleMessage1.setTo("[email protected]");
 		SimpleMailMessage simpleMessage2 = new SimpleMailMessage();
 		simpleMessage2.setTo("[email protected]");
-		sender.send(new SimpleMailMessage[] {simpleMessage1, simpleMessage2});
+		sender.send(simpleMessage1, simpleMessage2);
 
 		assertEquals("host", sender.transport.getConnectedHost());
 		assertEquals("username", sender.transport.getConnectedUsername());
@@ -152,7 +152,7 @@ public void testJavaMailSenderWithMimeMessages() throws MessagingException {
 		mimeMessage1.setRecipient(Message.RecipientType.TO, new InternetAddress("[email protected]"));
 		MimeMessage mimeMessage2 = sender.createMimeMessage();
 		mimeMessage2.setRecipient(Message.RecipientType.TO, new InternetAddress("[email protected]"));
-		sender.send(new MimeMessage[] {mimeMessage1, mimeMessage2});
+		sender.send(mimeMessage1, mimeMessage2);
 
 		assertEquals("host", sender.transport.getConnectedHost());
 		assertEquals("username", sender.transport.getConnectedUsername());
@@ -210,7 +210,7 @@ public void prepare(MimeMessage mimeMessage) throws MessagingException {
 				messages.add(mimeMessage);
 			}
 		};
-		sender.send(new MimeMessagePreparator[] {preparator1, preparator2});
+		sender.send(preparator1, preparator2);
 
 		assertEquals("host", sender.transport.getConnectedHost());
 		assertEquals("username", sender.transport.getConnectedUsername());
@@ -425,7 +425,7 @@ public void testFailedSimpleMessage() throws Exception {
 		simpleMessage2.setTo("[email protected]");
 
 		try {
-			sender.send(new SimpleMailMessage[] {simpleMessage1, simpleMessage2});
+			sender.send(simpleMessage1, simpleMessage2);
 		}
 		catch (MailSendException ex) {
 			ex.printStackTrace();
@@ -456,7 +456,7 @@ public void testFailedMimeMessage() throws Exception {
 		mimeMessage2.setRecipient(Message.RecipientType.TO, new InternetAddress("[email protected]"));
 
 		try {
-			sender.send(new MimeMessage[] {mimeMessage1, mimeMessage2});
+			sender.send(mimeMessage1, mimeMessage2);
 		}
 		catch (MailSendException ex) {
 			ex.printStackTrace();
@@ -537,6 +537,7 @@ public void connect(String host, int port, String username, String password) thr
 			this.connectedPort = port;
 			this.connectedUsername = username;
 			this.connectedPassword = password;
+			setConnected(true);
 		}
 
 		@Override
@@ -552,9 +553,7 @@ public void sendMessage(Message message, Address[] addresses) throws MessagingEx
 			if ("fail".equals(message.getSubject())) {
 				throw new MessagingException("failed");
 			}
-			List<Address> addr1 = Arrays.asList(message.getAllRecipients());
-			List<Address> addr2 = Arrays.asList(addresses);
-			if (!addr1.equals(addr2)) {
+			if (!ObjectUtils.nullSafeEquals(addresses, message.getAllRecipients())) {
 				throw new MessagingException("addresses not correct");
 			}
 			if (message.getSentDate() == null) {