From ec93f21809579fd6ab4ef11eb3fb0f90a8a046a3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 10 Jun 2010 21:06:13 +0000 Subject: [PATCH] MailSendException includes all messages as failed messages in case of a connect failure (SPR-7245) git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@3408 50f2f4bb-b051-0410-bef5-90022cba6387 --- .../mail/MailSendException.java | 35 +++++++-- .../mail/javamail/JavaMailSenderImpl.java | 73 ++++++++++++------- .../mail/javamail/JavaMailSenderTests.java | 31 +++++++- 3 files changed, 100 insertions(+), 39 deletions(-) diff --git a/org.springframework.context.support/src/main/java/org/springframework/mail/MailSendException.java b/org.springframework.context.support/src/main/java/org/springframework/mail/MailSendException.java index 9c69beb3da7..faefe649827 100644 --- a/org.springframework.context.support/src/main/java/org/springframework/mail/MailSendException.java +++ b/org.springframework.context.support/src/main/java/org/springframework/mail/MailSendException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2010 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. @@ -60,15 +60,29 @@ public class MailSendException extends MailException { * messages that failed as keys, and the thrown exceptions as values. *

The messages should be the same that were originally passed * to the invoked send method. + * @param msg the detail message + * @param cause the root cause from the mail API in use * @param failedMessages Map of failed messages as keys and thrown * exceptions as values */ - public MailSendException(Map failedMessages) { - super(null); + public MailSendException(String msg, Throwable cause, Map failedMessages) { + super(msg, cause); this.failedMessages = new LinkedHashMap(failedMessages); this.messageExceptions = failedMessages.values().toArray(new Exception[failedMessages.size()]); } + /** + * Constructor for registration of failed messages, with the + * messages that failed as keys, and the thrown exceptions as values. + *

The messages should be the same that were originally passed + * to the invoked send method. + * @param failedMessages Map of failed messages as keys and thrown + * exceptions as values + */ + public MailSendException(Map failedMessages) { + this(null, null, failedMessages); + } + /** * Return a Map with the failed messages as keys, and the thrown exceptions @@ -111,7 +125,12 @@ public class MailSendException extends MailException { return super.getMessage(); } else { - StringBuilder sb = new StringBuilder("Failed messages: "); + StringBuilder sb = new StringBuilder(); + String baseMessage = super.getMessage(); + if (baseMessage != null) { + sb.append(baseMessage).append(". "); + } + sb.append("Failed messages: "); for (int i = 0; i < this.messageExceptions.length; i++) { Exception subEx = this.messageExceptions[i]; sb.append(subEx.toString()); @@ -129,8 +148,8 @@ public class MailSendException extends MailException { return super.toString(); } else { - StringBuilder sb = new StringBuilder(getClass().getName()); - sb.append("; nested exceptions (").append(this.messageExceptions.length).append(") are:"); + StringBuilder sb = new StringBuilder(super.toString()); + sb.append("; message exceptions (").append(this.messageExceptions.length).append(") are:"); for (int i = 0; i < this.messageExceptions.length; i++) { Exception subEx = this.messageExceptions[i]; sb.append('\n').append("Failed message ").append(i + 1).append(": "); @@ -146,7 +165,7 @@ public class MailSendException extends MailException { super.printStackTrace(ps); } else { - ps.println(getClass().getName() + "; nested exception details (" + + ps.println(super.toString() + "; message exception details (" + this.messageExceptions.length + ") are:"); for (int i = 0; i < this.messageExceptions.length; i++) { Exception subEx = this.messageExceptions[i]; @@ -162,7 +181,7 @@ public class MailSendException extends MailException { super.printStackTrace(pw); } else { - pw.println(getClass().getName() + "; nested exception details (" + + pw.println(super.toString() + "; message exception details (" + this.messageExceptions.length + ") are:"); for (int i = 0; i < this.messageExceptions.length; i++) { Exception subEx = this.messageExceptions[i]; diff --git a/org.springframework.context.support/src/main/java/org/springframework/mail/javamail/JavaMailSenderImpl.java b/org.springframework.context.support/src/main/java/org/springframework/mail/javamail/JavaMailSenderImpl.java index 96611dda988..cea14b5f302 100644 --- a/org.springframework.context.support/src/main/java/org/springframework/mail/javamail/JavaMailSenderImpl.java +++ b/org.springframework.context.support/src/main/java/org/springframework/mail/javamail/JavaMailSenderImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2010 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. @@ -24,7 +24,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Properties; - import javax.activation.FileTypeMap; import javax.mail.AuthenticationFailedException; import javax.mail.MessagingException; @@ -383,40 +382,60 @@ public class JavaMailSenderImpl implements JavaMailSender { */ protected void doSend(MimeMessage[] mimeMessages, Object[] originalMessages) throws MailException { Map failedMessages = new LinkedHashMap(); + + Transport transport; try { - Transport transport = getTransport(getSession()); + transport = getTransport(getSession()); transport.connect(getHost(), getPort(), getUsername(), getPassword()); - try { - for (int i = 0; i < mimeMessages.length; i++) { - MimeMessage mimeMessage = mimeMessages[i]; - try { - if (mimeMessage.getSentDate() == null) { - mimeMessage.setSentDate(new Date()); - } - String messageId = mimeMessage.getMessageID(); - mimeMessage.saveChanges(); - if (messageId != null) { - // Preserve explicitly specified message id... - mimeMessage.setHeader(HEADER_MESSAGE_ID, messageId); - } - transport.sendMessage(mimeMessage, mimeMessage.getAllRecipients()); + } + 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); + } + + try { + for (int i = 0; i < mimeMessages.length; i++) { + MimeMessage mimeMessage = mimeMessages[i]; + try { + if (mimeMessage.getSentDate() == null) { + mimeMessage.setSentDate(new Date()); } - catch (MessagingException ex) { - Object original = (originalMessages != null ? originalMessages[i] : mimeMessage); - failedMessages.put(original, ex); + String messageId = mimeMessage.getMessageID(); + mimeMessage.saveChanges(); + if (messageId != null) { + // Preserve explicitly specified message id... + mimeMessage.setHeader(HEADER_MESSAGE_ID, messageId); } + transport.sendMessage(mimeMessage, mimeMessage.getAllRecipients()); + } + catch (MessagingException ex) { + Object original = (originalMessages != null ? originalMessages[i] : mimeMessage); + failedMessages.put(original, ex); } } - finally { + } + finally { + try { transport.close(); } + catch (MessagingException ex) { + if (!failedMessages.isEmpty()) { + throw new MailSendException("Failed to close server connection after message failures", ex, + failedMessages); + } + else { + throw new MailSendException("Failed to close server connection after message sending", ex); + } + } } - catch (AuthenticationFailedException ex) { - throw new MailAuthenticationException(ex); - } - catch (MessagingException ex) { - throw new MailSendException("Mail server connection failed", ex); - } + if (!failedMessages.isEmpty()) { throw new MailSendException(failedMessages); } diff --git a/org.springframework.context.support/src/test/java/org/springframework/mail/javamail/JavaMailSenderTests.java b/org.springframework.context.support/src/test/java/org/springframework/mail/javamail/JavaMailSenderTests.java index 99458d1a89c..b0deb96a6a2 100644 --- a/org.springframework.context.support/src/test/java/org/springframework/mail/javamail/JavaMailSenderTests.java +++ b/org.springframework.context.support/src/test/java/org/springframework/mail/javamail/JavaMailSenderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2006 the original author or authors. + * Copyright 2002-2010 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. @@ -22,7 +22,6 @@ import java.util.Arrays; import java.util.Date; import java.util.List; import java.util.Properties; - import javax.activation.FileTypeMap; import javax.mail.Address; import javax.mail.Message; @@ -381,8 +380,29 @@ public class JavaMailSenderTests extends TestCase { } catch (MailSendException ex) { // expected + ex.printStackTrace(); assertTrue(ex.getFailedMessages() != null); - assertTrue(ex.getFailedMessages().isEmpty()); + assertEquals(1, ex.getFailedMessages().size()); + assertSame(simpleMessage1, ex.getFailedMessages().keySet().iterator().next()); + assertSame(ex.getCause(), ex.getFailedMessages().values().iterator().next()); + } + } + + public void testFailedMailServerClose() throws Exception { + MockJavaMailSender sender = new MockJavaMailSender(); + sender.setHost(""); + sender.setUsername("username"); + sender.setPassword("password"); + SimpleMailMessage simpleMessage1 = new SimpleMailMessage(); + try { + sender.send(simpleMessage1); + fail("Should have thrown MailSendException"); + } + catch (MailSendException ex) { + // expected + ex.printStackTrace(); + assertTrue(ex.getFailedMessages() != null); + assertEquals(0, ex.getFailedMessages().size()); } } @@ -515,6 +535,9 @@ public class JavaMailSenderTests extends TestCase { @Override public synchronized void close() throws MessagingException { + if ("".equals(connectedHost)) { + throw new MessagingException("close failure"); + } this.closeCalled = true; } @@ -531,7 +554,7 @@ public class JavaMailSenderTests extends TestCase { if (message.getSentDate() == null) { throw new MessagingException("No sentDate specified"); } - if (message.getSubject() != null && message.getSubject().indexOf("custom") != -1) { + if (message.getSubject() != null && message.getSubject().contains("custom")) { assertEquals(new Date(2005, 3, 1), message.getSentDate()); } this.sentMessages.add(message);