Fix mail lock race condition & Sonar smells (#2721)

* Fix mail lock race condition & Sonar smells

https://build.spring.io/browse/INT-MASTERSPRING40-599/

* The `folderReadLock` might not be re-locked when `openFolder()`
throws an exception
* Fix all the Sonar smells for mail module
* Optimize all the dynamic `Assert` messages to `Supplier`
* Refactor `MailReceiverFactoryBean` to be based on the `AbstractFactoryBean`

* * Fix new reported Sonar smells

* * Log error during cancel idle state in the `ImapMailReceiver`
* Remove commented code in the `MailTransportUtils`

spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java

@@ -25,7 +25,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.mail.Authenticator;
@@ -72,7 +71,7 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl
 
 	private final URLName url;
 
-	private final ReadWriteLock folderLock = new ReentrantReadWriteLock();
+	private final ReentrantReadWriteLock folderLock = new ReentrantReadWriteLock();
 
 	private final Lock folderReadLock = this.folderLock.readLock();
 
@@ -340,7 +339,7 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl
 
 	@Override
 	public Object[] receive() throws javax.mail.MessagingException {
-		this.folderReadLock.lock();
+		this.folderReadLock.lock(); // NOSONAR - guarded with the getReadHoldCount()
 		try {
 			try {
 				Folder folderToCheck = getFolder();
@@ -362,7 +361,9 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl
 			}
 		}
 		finally {
-			this.folderReadLock.unlock();
+			if (this.folderLock.getReadHoldCount() > 0) {
+				this.folderReadLock.unlock();
+			}
 		}
 	}
 
@@ -416,22 +417,17 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl
 	private Object extractContent(MimeMessage message, Map<String, Object> headers) {
 		Object content;
 		try {
-			MimeMessage theMessage;
+			MimeMessage theMessage = message;
 			if (this.simpleContent) {
 				theMessage = new IntegrationMimeMessage(message);
 			}
-			else {
-				theMessage = message;
-			}
 			content = theMessage.getContent();
 			if (content instanceof String) {
+				headers.put(MessageHeaders.CONTENT_TYPE, "text/plain");
 				String mailContentType = (String) headers.get(MailHeaders.CONTENT_TYPE);
 				if (mailContentType != null && mailContentType.toLowerCase().startsWith("text")) {
 					headers.put(MessageHeaders.CONTENT_TYPE, mailContentType);
 				}
-				else {
-					headers.put(MessageHeaders.CONTENT_TYPE, "text/plain");
-				}
 			}
 			else if (content instanceof InputStream) {
 				ByteArrayOutputStream baos = new ByteArrayOutputStream();

spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapIdleChannelAdapter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -154,7 +154,7 @@ public class ImapIdleChannelAdapter extends MessageProducerSupport implements Be
 
 	@Override // guarded by super#lifecycleLock
 	protected void doStart() {
-		final TaskScheduler scheduler = this.getTaskScheduler();
+		TaskScheduler scheduler = getTaskScheduler();
 		Assert.notNull(scheduler, "'taskScheduler' must not be null");
 		if (this.sendingTaskExecutor == null) {
 			this.sendingTaskExecutor = Executors.newFixedThreadPool(1);
@@ -182,39 +182,8 @@ public class ImapIdleChannelAdapter extends MessageProducerSupport implements Be
 		}
 	}
 
-	private Runnable createMessageSendingTask(final Object mailMessage) {
-		Runnable sendingTask = () -> {
-			@SuppressWarnings("unchecked")
-			org.springframework.messaging.Message<?> message =
-					mailMessage instanceof Message
-							? getMessageBuilderFactory().withPayload(mailMessage).build()
-							: (org.springframework.messaging.Message<Object>) mailMessage;
-
-			if (TransactionSynchronizationManager.isActualTransactionActive()) {
-				if (ImapIdleChannelAdapter.this.transactionSynchronizationFactory != null) {
-					TransactionSynchronization synchronization =
-							ImapIdleChannelAdapter.this.transactionSynchronizationFactory
-									.create(ImapIdleChannelAdapter.this);
-					if (synchronization != null) {
-						TransactionSynchronizationManager.registerSynchronization(synchronization);
-
-						if (synchronization instanceof IntegrationResourceHolderSynchronization
-								&& !TransactionSynchronizationManager.hasResource(ImapIdleChannelAdapter.this)) {
-
-							TransactionSynchronizationManager.bindResource(ImapIdleChannelAdapter.this,
-									((IntegrationResourceHolderSynchronization) synchronization).getResourceHolder());
-						}
-
-						Object resourceHolder =
-								TransactionSynchronizationManager.getResource(ImapIdleChannelAdapter.this);
-						if (resourceHolder instanceof IntegrationResourceHolder) {
-							((IntegrationResourceHolder) resourceHolder).setMessage(message);
-						}
-					}
-				}
-			}
-			sendMessage(message);
-		};
+	private Runnable createMessageSendingTask(Object mailMessage) {
+		Runnable sendingTask = prepareSendingTask(mailMessage);
 
 		// wrap in the TX proxy if necessary
 		if (!CollectionUtils.isEmpty(this.adviceChain)) {
@@ -229,6 +198,38 @@ public class ImapIdleChannelAdapter extends MessageProducerSupport implements Be
 		return sendingTask;
 	}
 
+	private Runnable prepareSendingTask(Object mailMessage) {
+		return () -> {
+			@SuppressWarnings("unchecked")
+			org.springframework.messaging.Message<?> message =
+					mailMessage instanceof Message
+							? getMessageBuilderFactory().withPayload(mailMessage).build()
+							: (org.springframework.messaging.Message<Object>) mailMessage;
+
+			if (TransactionSynchronizationManager.isActualTransactionActive()
+					&& this.transactionSynchronizationFactory != null) {
+
+				TransactionSynchronization synchronization = this.transactionSynchronizationFactory.create(this);
+				if (synchronization != null) {
+					TransactionSynchronizationManager.registerSynchronization(synchronization);
+
+					if (synchronization instanceof IntegrationResourceHolderSynchronization
+							&& !TransactionSynchronizationManager.hasResource(this)) {
+
+						TransactionSynchronizationManager.bindResource(this,
+								((IntegrationResourceHolderSynchronization) synchronization).getResourceHolder());
+					}
+
+					Object resourceHolder = TransactionSynchronizationManager.getResource(this);
+					if (resourceHolder instanceof IntegrationResourceHolder) {
+						((IntegrationResourceHolder) resourceHolder).setMessage(message);
+					}
+				}
+			}
+			sendMessage(message);
+		};
+	}
+
 	private void publishException(Exception e) {
 		if (this.applicationEventPublisher != null) {
 			this.applicationEventPublisher.publishEvent(new ImapIdleExceptionEvent(e));
@@ -262,6 +263,7 @@ public class ImapIdleChannelAdapter extends MessageProducerSupport implements Be
 				publishException(e);
 			}
 		}
+
 	}
 
 

spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java

@@ -59,6 +59,8 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 
 	private static final int DEFAULT_CANCEL_IDLE_INTERVAL = 120000;
 
+	private static final String PROTOCOL = "imap";
+
 	private final MessageCountListener messageCountListener = new SimpleMessageCountListener();
 
 	private final IdleCanceler idleCanceler = new IdleCanceler();
@@ -77,17 +79,17 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 
 	public ImapMailReceiver() {
 		super();
-		this.setProtocol("imap");
+		setProtocol(PROTOCOL);
 	}
 
 	public ImapMailReceiver(String url) {
 		super(url);
 		if (url != null) {
-			Assert.isTrue(url.toLowerCase().startsWith("imap"),
+			Assert.isTrue(url.toLowerCase().startsWith(PROTOCOL),
 					"URL must start with 'imap' for the IMAP Mail receiver.");
 		}
 		else {
-			this.setProtocol("imap");
+			setProtocol(PROTOCOL);
 		}
 	}
 
@@ -126,7 +128,7 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 	 * @since 3.0.5
 	 */
 	public void setCancelIdleInterval(long cancelIdleInterval) {
-		this.cancelIdleInterval = cancelIdleInterval * 1000;
+		this.cancelIdleInterval = cancelIdleInterval * 1000; // NOSONAR - convert seconds to milliseconds
 	}
 
 	@Override
@@ -140,7 +142,7 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 			this.isInternalScheduler = true;
 		}
 		Properties javaMailProperties = getJavaMailProperties();
-		for (String name : new String[]{"imap", "imaps"}) {
+		for (String name : new String[] { PROTOCOL, "imaps" }) {
 			String peek = "mail." + name + ".peek";
 			if (javaMailProperties.getProperty(peek) == null) {
 				javaMailProperties.setProperty(peek, "true");
@@ -154,6 +156,9 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 		if (this.isInternalScheduler) {
 			((ThreadPoolTaskScheduler) this.scheduler).shutdown();
 		}
+		if (this.pingTask != null) {
+			this.pingTask.cancel(true);
+		}
 	}
 
 	/**
@@ -162,18 +167,16 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 	 * @throws MessagingException Any MessagingException.
 	 */
 	public void waitForNewMessages() throws MessagingException {
-		this.openFolder();
-		Folder folder = this.getFolder();
+		openFolder();
+		Folder folder = getFolder();
 		Assert.state(folder instanceof IMAPFolder,
-				"folder is not an instance of [" + IMAPFolder.class.getName() + "]");
+				() -> "folder is not an instance of [" + IMAPFolder.class.getName() + "]");
 		IMAPFolder imapFolder = (IMAPFolder) folder;
 		if (imapFolder.hasNewMessages()) {
 			return;
 		}
-		else if (!folder.getPermanentFlags().contains(Flags.Flag.RECENT)) {
-			if (searchForNewMessages().length > 0) {
-				return;
-			}
+		else if (!folder.getPermanentFlags().contains(Flags.Flag.RECENT) && searchForNewMessages().length > 0) {
+			return;
 		}
 		imapFolder.addMessageCountListener(this.messageCountListener);
 		try {
@@ -203,11 +206,11 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 	 */
 	@Override
 	protected Message[] searchForNewMessages() throws MessagingException {
-		Flags supportedFlags = this.getFolder().getPermanentFlags();
-		SearchTerm searchTerm = this.compileSearchTerms(supportedFlags);
-		Folder folder = this.getFolder();
-		if (folder.isOpen()) {
-			return nullSafeMessages(searchTerm != null ? folder.search(searchTerm) : folder.getMessages());
+		Folder folderToUse = getFolder();
+		Flags supportedFlags = folderToUse.getPermanentFlags();
+		SearchTerm searchTerm = compileSearchTerms(supportedFlags);
+		if (folderToUse.isOpen()) {
+			return nullSafeMessages(searchTerm != null ? folderToUse.search(searchTerm) : folderToUse.getMessages());
 		}
 		throw new MessagingException("Folder is closed");
 	}
@@ -257,9 +260,11 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 					folder.isOpen(); // resets idle state
 				}
 			}
-			catch (Exception ignore) {
+			catch (Exception ex) {
+				logger.error("Error during resetting idle state.", ex);
 			}
 		}
+
 	}
 
 	/**
@@ -327,28 +332,35 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 			}
 
 			if (!recentFlagSupported) {
-				NotTerm notFlagged = null;
-				if (folder.getPermanentFlags().contains(Flags.Flag.USER)) {
+				searchTerm = applyTermsWhenNoRecentFlag(folder, searchTerm);
+			}
+			return searchTerm;
+		}
+
+		private SearchTerm applyTermsWhenNoRecentFlag(Folder folder, SearchTerm searchTerm) {
+			NotTerm notFlagged = null;
+			if (folder.getPermanentFlags().contains(Flag.USER)) {
+				if (logger.isDebugEnabled()) {
 					logger.debug("This email server does not support RECENT flag, but it does support " +
 							"USER flags which will be used to prevent duplicates during email fetch." +
 							" This receiver instance uses flag: " + getUserFlag());
-					Flags siFlags = new Flags();
-					siFlags.add(getUserFlag());
-					notFlagged = new NotTerm(new FlagTerm(siFlags, true));
-				}
-				else {
-					logger.debug("This email server does not support RECENT or USER flags. " +
-							"System flag 'Flag.FLAGGED' will be used to prevent duplicates during email fetch.");
-					notFlagged = new NotTerm(new FlagTerm(new Flags(Flags.Flag.FLAGGED), true));
-				}
-				if (searchTerm == null) {
-					searchTerm = notFlagged;
-				}
-				else {
-					searchTerm = new AndTerm(searchTerm, notFlagged);
 				}
+				Flags siFlags = new Flags();
+				siFlags.add(getUserFlag());
+				notFlagged = new NotTerm(new FlagTerm(siFlags, true));
+			}
+			else {
+				logger.debug("This email server does not support RECENT or USER flags. " +
+						"System flag 'Flag.FLAGGED' will be used to prevent duplicates during email fetch.");
+				notFlagged = new NotTerm(new FlagTerm(new Flags(Flag.FLAGGED), true));
+			}
+
+			if (searchTerm == null) {
+				return notFlagged;
+			}
+			else {
+				return new AndTerm(searchTerm, notFlagged);
 			}
-			return searchTerm;
 		}
 
 	}

spring-integration-mail/src/main/java/org/springframework/integration/mail/MailTransportUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2007-2018 the original author or authors.
+ * Copyright 2007-2019 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.
@@ -31,16 +31,17 @@ import org.springframework.util.StringUtils;
  *
  * @author Arjen Poutsma
  * @author Mark Fisher
+ * @author Artem Bilan
  */
 public abstract class MailTransportUtils {
 
-	private static final Log logger = LogFactory.getLog(MailTransportUtils.class);
+	private static final Log LOGGER = LogFactory.getLog(MailTransportUtils.class);
 
 
 	/**
-	 * Close the given JavaMail Service and ignore any thrown exception. This is useful for typical <code>finally</code>
+	 * Close the given JavaMail Service and ignore any thrown exception. This is useful for typical
+	 * <code>finally</code>
 	 * blocks in manual JavaMail code.
-	 *
 	 * @param service the JavaMail Service to close (may be <code>null</code>)
 	 * @see javax.mail.Transport
 	 * @see javax.mail.Store
@@ -51,26 +52,14 @@ public abstract class MailTransportUtils {
 				service.close();
 			}
 			catch (MessagingException ex) {
-				logger.debug("Could not close JavaMail Service", ex);
+				LOGGER.debug("Could not close JavaMail Service", ex);
 			}
 		}
 	}
 
-//    /**
-//     * Close the given JavaMail Folder and ignore any thrown exception. This is
-//     * useful for typical <code>finally</code> blocks in manual JavaMail code.
-//     *
-//     * @param folder the JavaMail Folder to close (may be <code>null</code>)
-//     */
-//
-//    public static void closeFolder(Folder folder) {
-//        closeFolder(folder, false);
-//    }
-
 	/**
 	 * Close the given JavaMail Folder and ignore any thrown exception. This is
 	 * useful for typical <code>finally</code> blocks in manual JavaMail code.
-	 *
 	 * @param folder  the JavaMail Folder to close (may be <code>null</code>)
 	 * @param expunge whether all deleted messages should be expunged from the folder
 	 */
@@ -80,11 +69,7 @@ public abstract class MailTransportUtils {
 				folder.close(expunge);
 			}
 			catch (MessagingException ex) {
-				logger.debug("Could not close JavaMail Folder", ex);
-			}
-			catch (NullPointerException ex) {
-				// JavaMail prior to 1.4.1 may throw this
-				logger.debug("Could not close JavaMail Folder", ex);
+				LOGGER.debug("Could not close JavaMail Folder", ex);
 			}
 		}
 	}
@@ -92,7 +77,6 @@ public abstract class MailTransportUtils {
 	/**
 	 * Returns a string representation of the given {@link URLName}, where the
 	 * password has been protected.
-	 *
 	 * @param name The URL name.
 	 * @return The result with password protection.
 	 */
@@ -104,7 +88,7 @@ public abstract class MailTransportUtils {
 		int port = name.getPort();
 		String file = name.getFile();
 		String ref = name.getRef();
-		StringBuffer tempURL = new StringBuffer();
+		StringBuilder tempURL = new StringBuilder();
 		if (protocol != null) {
 			tempURL.append(protocol).append(':');
 		}
@@ -121,7 +105,7 @@ public abstract class MailTransportUtils {
 				tempURL.append(host);
 			}
 			if (port != -1) {
-				tempURL.append(':').append(Integer.toString(port));
+				tempURL.append(':').append(port);
 			}
 			if (StringUtils.hasLength(file)) {
 				tempURL.append('/');

spring-integration-mail/src/main/java/org/springframework/integration/mail/Pop3MailReceiver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.integration.mail;
 
+import javax.mail.Folder;
 import javax.mail.Message;
 import javax.mail.MessagingException;
 import javax.mail.URLName;
@@ -32,18 +33,20 @@ import org.springframework.util.Assert;
  */
 public class Pop3MailReceiver extends AbstractMailReceiver {
 
+	public static final String PROTOCOL = "pop3";
+
 	public Pop3MailReceiver() {
 		super();
-		this.setProtocol("pop3");
+		setProtocol(PROTOCOL);
 	}
 
 	public Pop3MailReceiver(String url) {
 		super(url);
 		if (url != null) {
-			Assert.isTrue(url.startsWith("pop3"), "url must start with 'pop3'");
+			Assert.isTrue(url.startsWith(PROTOCOL), "url must start with 'pop3'");
 		}
 		else {
-			this.setProtocol("pop3");
+			setProtocol(PROTOCOL);
 		}
 	}
 
@@ -53,22 +56,22 @@ public class Pop3MailReceiver extends AbstractMailReceiver {
 	}
 
 	public Pop3MailReceiver(String host, int port, String username, String password) {
-		super(new URLName("pop3", host, port, "INBOX", username, password));
+		super(new URLName(PROTOCOL, host, port, "INBOX", username, password));
 	}
 
 
 	@Override
 	protected Message[] searchForNewMessages() throws MessagingException {
-		int messageCount = this.getFolder().getMessageCount();
+		Folder folderToUse = getFolder();
+		int messageCount = folderToUse.getMessageCount();
 		if (messageCount == 0) {
 			return new Message[0];
 		}
-		return this.getFolder().getMessages();
+		return folderToUse.getMessages();
 	}
 
 	/**
 	 * Deletes the given messages from this receiver's folder, and closes it to expunge deleted messages.
-	 *
 	 * @param messages the messages to delete
 	 * @throws MessagingException in case of JavaMail errors
 	 */
@@ -76,8 +79,9 @@ public class Pop3MailReceiver extends AbstractMailReceiver {
 	protected void deleteMessages(Message[] messages) throws MessagingException {
 		super.deleteMessages(messages);
 		// expunge deleted mails, and make sure we've retrieved them before closing the folder
-		for (int i = 0; i < messages.length; i++) {
-			new MimeMessage((MimeMessage) messages[i]);
+		for (Message message : messages) {
+			new MimeMessage((MimeMessage) message);
 		}
 	}
+
 }

spring-integration-mail/src/main/java/org/springframework/integration/mail/config/MailReceiverFactoryBean.java

@@ -23,14 +23,9 @@ import javax.mail.Session;
 import javax.mail.URLName;
 import javax.mail.internet.MimeMessage;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-
-import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
-import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.DisposableBean;
-import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.config.AbstractFactoryBean;
 import org.springframework.expression.Expression;
 import org.springframework.integration.mail.AbstractMailReceiver;
 import org.springframework.integration.mail.ImapMailReceiver;
@@ -38,6 +33,7 @@ import org.springframework.integration.mail.MailReceiver;
 import org.springframework.integration.mail.Pop3MailReceiver;
 import org.springframework.integration.mail.SearchTermStrategy;
 import org.springframework.integration.mapping.HeaderMapper;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
@@ -45,73 +41,71 @@ import org.springframework.util.StringUtils;
  * @author Mark Fisher
  * @author Oleg Zhurakousky
  * @author Gary Russell
+ * @author Artem Bilan
+ *
  * @since 1.0.3
  */
-public class MailReceiverFactoryBean implements FactoryBean<MailReceiver>, DisposableBean, BeanFactoryAware {
+public class MailReceiverFactoryBean extends AbstractFactoryBean<MailReceiver> {
 
-	protected final Log logger = LogFactory.getLog(this.getClass());
+	private String storeUri;
 
-	private volatile String storeUri;
+	private String protocol;
 
-	private volatile String protocol;
+	private Session session;
 
-	private volatile Session session;
+	private MailReceiver receiver;
 
-	private volatile MailReceiver receiver;
+	private Properties javaMailProperties;
 
-	private volatile Properties javaMailProperties;
-
-	private volatile Authenticator authenticator;
+	private Authenticator authenticator;
 
 	/**
 	 * Indicates whether retrieved messages should be deleted from the server.
 	 * This value will be <code>null</code> <i>unless</i> explicitly configured.
 	 */
-	private volatile Boolean shouldDeleteMessages = null;
+	private Boolean shouldDeleteMessages = null;
 
-	private volatile Boolean shouldMarkMessagesAsRead = null;
+	private Boolean shouldMarkMessagesAsRead = null;
 
-	private volatile int maxFetchSize = 1;
+	private int maxFetchSize = 1;
 
-	private volatile Expression selectorExpression;
+	private Expression selectorExpression;
 
-	private volatile SearchTermStrategy searchTermStrategy;
+	private SearchTermStrategy searchTermStrategy;
 
-	private volatile String userFlag;
+	private String userFlag;
 
-	private volatile BeanFactory beanFactory;
-
-	private volatile HeaderMapper<MimeMessage> headerMapper;
+	private HeaderMapper<MimeMessage> headerMapper;
 
 	private Boolean embeddedPartsAsBytes;
 
 	private Boolean simpleContent;
 
-	public void setStoreUri(String storeUri) {
+	public void setStoreUri(@Nullable String storeUri) {
 		this.storeUri = storeUri;
 	}
 
-	public void setProtocol(String protocol) {
+	public void setProtocol(@Nullable String protocol) {
 		this.protocol = protocol;
 	}
 
-	public void setSession(Session session) {
+	public void setSession(@Nullable Session session) {
 		this.session = session;
 	}
 
-	public void setJavaMailProperties(Properties javaMailProperties) {
+	public void setJavaMailProperties(@Nullable Properties javaMailProperties) {
 		this.javaMailProperties = javaMailProperties;
 	}
 
-	public void setAuthenticator(Authenticator authenticator) {
+	public void setAuthenticator(@Nullable Authenticator authenticator) {
 		this.authenticator = authenticator;
 	}
 
-	public void setShouldDeleteMessages(Boolean shouldDeleteMessages) {
+	public void setShouldDeleteMessages(@Nullable Boolean shouldDeleteMessages) {
 		this.shouldDeleteMessages = shouldDeleteMessages;
 	}
 
-	public void setShouldMarkMessagesAsRead(Boolean shouldMarkMessagesAsRead) {
+	public void setShouldMarkMessagesAsRead(@Nullable Boolean shouldMarkMessagesAsRead) {
 		this.shouldMarkMessagesAsRead = shouldMarkMessagesAsRead;
 	}
 
@@ -123,37 +117,32 @@ public class MailReceiverFactoryBean implements FactoryBean<MailReceiver>, Dispo
 		this.maxFetchSize = maxFetchSize;
 	}
 
-	public void setSelectorExpression(Expression selectorExpression) {
+	public void setSelectorExpression(@Nullable Expression selectorExpression) {
 		this.selectorExpression = selectorExpression;
 	}
 
-	public void setSearchTermStrategy(SearchTermStrategy searchTermStrategy) {
+	public void setSearchTermStrategy(@Nullable SearchTermStrategy searchTermStrategy) {
 		this.searchTermStrategy = searchTermStrategy;
 	}
 
-	public void setUserFlag(String userFlag) {
+	public void setUserFlag(@Nullable String userFlag) {
 		this.userFlag = userFlag;
 	}
 
-	public void setHeaderMapper(HeaderMapper<MimeMessage> headerMapper) {
+	public void setHeaderMapper(@Nullable HeaderMapper<MimeMessage> headerMapper) {
 		this.headerMapper = headerMapper;
 	}
 
-	public void setEmbeddedPartsAsBytes(Boolean embeddedPartsAsBytes) {
+	public void setEmbeddedPartsAsBytes(@Nullable Boolean embeddedPartsAsBytes) {
 		this.embeddedPartsAsBytes = embeddedPartsAsBytes;
 	}
 
-	public void setSimpleContent(Boolean simpleContent) {
+	public void setSimpleContent(@Nullable Boolean simpleContent) {
 		this.simpleContent = simpleContent;
 	}
 
 	@Override
-	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
-		this.beanFactory = beanFactory;
-	}
-
-	@Override
-	public MailReceiver getObject() throws Exception {
+	protected MailReceiver createInstance() {
 		if (this.receiver == null) {
 			this.receiver = this.createReceiver();
 		}
@@ -165,30 +154,8 @@ public class MailReceiverFactoryBean implements FactoryBean<MailReceiver>, Dispo
 		return (this.receiver != null) ? this.receiver.getClass() : MailReceiver.class;
 	}
 
-	@Override
-	public boolean isSingleton() {
-		return true;
-	}
-
-	private void verifyProtocol() {
-		if (StringUtils.hasText(this.storeUri)) {
-			URLName urlName = new URLName(this.storeUri);
-			if (this.protocol == null) {
-				this.protocol = urlName.getProtocol();
-			}
-			else {
-				Assert.isTrue(this.protocol.equals(urlName.getProtocol()),
-						"The provided 'protocol' does not match that in the 'storeUri'.");
-			}
-		}
-		else {
-			Assert.hasText(this.protocol, "Either the 'storeUri' or 'protocol' is required.");
-		}
-		Assert.hasText(this.protocol, "Unable to resolve protocol.");
-	}
-
-	private MailReceiver createReceiver() {
-		this.verifyProtocol();
+	private MailReceiver createReceiver() { // NOSONAR
+		verifyProtocol();
 		boolean isPop3 = this.protocol.toLowerCase().startsWith("pop3");
 		boolean isImap = this.protocol.toLowerCase().startsWith("imap");
 		Assert.isTrue(isPop3 || isImap, "the store URI must begin with 'pop3' or 'imap'");
@@ -196,8 +163,10 @@ public class MailReceiverFactoryBean implements FactoryBean<MailReceiver>, Dispo
 				? new Pop3MailReceiver(this.storeUri)
 				: new ImapMailReceiver(this.storeUri);
 		if (this.session != null) {
-			Assert.isNull(this.javaMailProperties, "JavaMail Properties are not allowed when a Session has been provided.");
-			Assert.isNull(this.authenticator, "A JavaMail Authenticator is not allowed when a Session has been provided.");
+			Assert.isNull(this.javaMailProperties,
+					"JavaMail Properties are not allowed when a Session has been provided.");
+			Assert.isNull(this.authenticator,
+					"A JavaMail Authenticator is not allowed when a Session has been provided.");
 			mailReceiver.setSession(this.session);
 		}
 		if (this.searchTermStrategy != null) {
@@ -222,15 +191,16 @@ public class MailReceiverFactoryBean implements FactoryBean<MailReceiver>, Dispo
 		}
 
 		if (isPop3) {
-			if (this.isShouldMarkMessagesAsRead() && this.logger.isWarnEnabled()) {
+			if (isShouldMarkMessagesAsRead()) {
 				this.logger.warn("Setting 'should-mark-messages-as-read' to 'true' while using POP3 has no effect");
 			}
 		}
-		else if (isImap) {
+		else {
 			((ImapMailReceiver) mailReceiver).setShouldMarkMessagesAsRead(this.shouldMarkMessagesAsRead);
 		}
-		if (this.beanFactory != null) {
-			mailReceiver.setBeanFactory(this.beanFactory);
+		BeanFactory beanFactory = getBeanFactory();
+		if (beanFactory != null) {
+			mailReceiver.setBeanFactory(beanFactory);
 		}
 		if (this.headerMapper != null) {
 			mailReceiver.setHeaderMapper(this.headerMapper);
@@ -245,10 +215,32 @@ public class MailReceiverFactoryBean implements FactoryBean<MailReceiver>, Dispo
 		return mailReceiver;
 	}
 
+	private void verifyProtocol() {
+		if (StringUtils.hasText(this.storeUri)) {
+			URLName urlName = new URLName(this.storeUri);
+			if (this.protocol == null) {
+				this.protocol = urlName.getProtocol();
+			}
+			else {
+				Assert.isTrue(this.protocol.equals(urlName.getProtocol()),
+						"The provided 'protocol' does not match that in the 'storeUri'.");
+			}
+		}
+		else {
+			Assert.hasText(this.protocol, "Either the 'storeUri' or 'protocol' is required.");
+		}
+		Assert.hasText(this.protocol, "Unable to resolve protocol.");
+	}
+
 	@Override
-	public void destroy() throws Exception {
+	public void destroy() {
 		if (this.receiver != null && this.receiver instanceof DisposableBean) {
-			((DisposableBean) this.receiver).destroy();
+			try {
+				((DisposableBean) this.receiver).destroy();
+			}
+			catch (Exception e) {
+				throw new IllegalStateException(e);
+			}
 		}
 	}
 

spring-integration-mail/src/main/java/org/springframework/integration/mail/dsl/ImapIdleChannelAdapterSpec.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2018 the original author or authors.
+ * Copyright 2014-2019 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.
@@ -65,7 +65,7 @@ public class ImapIdleChannelAdapterSpec
 
 	private final List<Advice> adviceChain = new LinkedList<>();
 
-	protected final boolean externalReceiver;
+	protected final boolean externalReceiver; // NOSONAR
 
 	private boolean sessionProvided;
 
@@ -106,7 +106,8 @@ public class ImapIdleChannelAdapterSpec
 	}
 
 	private void assertReceiver() {
-		Assert.state(!this.externalReceiver, "An external 'receiver' [" + this.receiver + "] can't be modified.");
+		Assert.state(!this.externalReceiver,
+				() -> "An external 'receiver' [" + this.receiver + "] can't be modified.");
 	}
 
 	/**
@@ -118,7 +119,7 @@ public class ImapIdleChannelAdapterSpec
 	 * @see FunctionExpression
 	 */
 	public ImapIdleChannelAdapterSpec selector(Function<MimeMessage, Boolean> selectorFunction) {
-		return selectorExpression(new FunctionExpression<MimeMessage>(selectorFunction));
+		return selectorExpression(new FunctionExpression<>(selectorFunction));
 	}
 
 	/**
@@ -277,6 +278,7 @@ public class ImapIdleChannelAdapterSpec
 	 */
 	public ImapIdleChannelAdapterSpec transactionSynchronizationFactory(
 			TransactionSynchronizationFactory transactionSynchronizationFactory) {
+
 		this.target.setTransactionSynchronizationFactory(transactionSynchronizationFactory);
 		return this;
 	}

spring-integration-mail/src/main/java/org/springframework/integration/mail/dsl/MailInboundChannelAdapterSpec.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2018 the original author or authors.
+ * Copyright 2014-2019 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.
@@ -45,6 +45,7 @@ import org.springframework.util.Assert;
  *
  * @author Gary Russell
  * @author Artem Bilan
+ *
  * @since 5.0
  */
 public abstract class
@@ -52,9 +53,9 @@ MailInboundChannelAdapterSpec<S extends MailInboundChannelAdapterSpec<S, R>, R e
 		extends MessageSourceSpec<S, MailReceivingMessageSource>
 		implements ComponentsRegistration {
 
-	protected final R receiver;
+	protected final R receiver; // NOSONAR
 
-	protected final boolean externalReceiver;
+	protected final boolean externalReceiver; // NOSONAR
 
 	private boolean sessionProvided;
 
@@ -81,7 +82,8 @@ MailInboundChannelAdapterSpec<S extends MailInboundChannelAdapterSpec<S, R>, R e
 	}
 
 	protected void assertReceiver() {
-		Assert.state(!this.externalReceiver, "An external 'receiver' [" + this.receiver + "] can't be modified.");
+		Assert.state(!this.externalReceiver,
+				() -> "An external 'receiver' [" + this.receiver + "] can't be modified.");
 	}
 
 	/**
@@ -107,7 +109,7 @@ MailInboundChannelAdapterSpec<S extends MailInboundChannelAdapterSpec<S, R>, R e
 	 */
 	public S selector(Function<MimeMessage, Boolean> selectorFunction) {
 		assertReceiver();
-		this.receiver.setSelectorExpression(new FunctionExpression<MimeMessage>(selectorFunction));
+		this.receiver.setSelectorExpression(new FunctionExpression<>(selectorFunction));
 		return _this();
 	}
 
@@ -227,8 +229,7 @@ MailInboundChannelAdapterSpec<S extends MailInboundChannelAdapterSpec<S, R>, R e
 	 * {@link javax.mail.Multipart} content is rendered as a byte[] in the payload.
 	 * Otherwise, leave as a {@link javax.mail.Part}. These objects are not suitable for
 	 * downstream serialization. Default: true.
-	 * <p>
-	 * This has no effect if there is no header mapper, in that case the payload is the
+	 * <p> This has no effect if there is no header mapper, in that case the payload is the
 	 * {@link MimeMessage}.
 	 * @param embeddedPartsAsBytes the embeddedPartsAsBytes to set.
 	 * @return the spec.

spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/AbstractMailMessageTransformer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -18,9 +18,6 @@ package org.springframework.integration.mail.transformer;
 
 import java.util.Map;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.integration.mail.support.MailUtils;
@@ -43,13 +40,11 @@ import org.springframework.messaging.Message;
 public abstract class AbstractMailMessageTransformer<T> implements Transformer,
 		BeanFactoryAware {
 
-	protected final Log logger = LogFactory.getLog(this.getClass());
+	private BeanFactory beanFactory;
 
-	private volatile BeanFactory beanFactory;
+	private MessageBuilderFactory messageBuilderFactory = new DefaultMessageBuilderFactory();
 
-	private volatile MessageBuilderFactory messageBuilderFactory = new DefaultMessageBuilderFactory();
-
-	private volatile boolean messageBuilderFactorySet;
+	private boolean messageBuilderFactorySet;
 
 
 	@Override
@@ -71,11 +66,11 @@ public abstract class AbstractMailMessageTransformer<T> implements Transformer,
 	public Message<?> transform(Message<?> message) {
 		Object payload = message.getPayload();
 		if (!(payload instanceof javax.mail.Message)) {
-			throw new MessageTransformationException(message, this.getClass().getSimpleName()
+			throw new MessageTransformationException(message, getClass().getSimpleName()
 					+ " requires a javax.mail.Message payload");
 		}
 		javax.mail.Message mailMessage = (javax.mail.Message) payload;
-		AbstractIntegrationMessageBuilder<T> builder = null;
+		AbstractIntegrationMessageBuilder<T> builder;
 		try {
 			builder = this.doTransform(mailMessage);
 		}
@@ -85,11 +80,12 @@ public abstract class AbstractMailMessageTransformer<T> implements Transformer,
 		if (builder == null) {
 			throw new MessageTransformationException(message, "failed to transform mail message");
 		}
-		builder.copyHeaders(this.extractHeaderMapFromMailMessage(mailMessage));
+		builder.copyHeaders(extractHeaderMapFromMailMessage(mailMessage));
 		return builder.build();
 	}
 
-	protected abstract AbstractIntegrationMessageBuilder<T> doTransform(javax.mail.Message mailMessage) throws Exception;
+	protected abstract AbstractIntegrationMessageBuilder<T> doTransform(javax.mail.Message mailMessage)
+			throws Exception; // NOSONAR
 
 
 	private Map<String, Object> extractHeaderMapFromMailMessage(javax.mail.Message mailMessage) {

spring-integration-mail/src/main/java/org/springframework/integration/mail/transformer/MailToStringTransformer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -33,26 +33,27 @@ import org.springframework.util.Assert;
  *
  * @author Mark Fisher
  * @author Gary Russell
+ * @author Artem Bilan
  */
 public class MailToStringTransformer extends AbstractMailMessageTransformer<String> {
 
-	private volatile String charset = "UTF-8";
-
+	private String charset = "UTF-8";
 
 	/**
 	 * Specify the name of the Charset to use when converting from bytes.
 	 * The default is UTF-8.
-	 *
 	 * @param charset The charset.
 	 */
 	public void setCharset(String charset) {
 		Assert.notNull(charset, "charset must not be null");
-		Assert.isTrue(Charset.isSupported(charset), "unsupported charset '" + charset + "'");
+		Assert.isTrue(Charset.isSupported(charset), () -> "unsupported charset '" + charset + "'");
 		this.charset = charset;
 	}
 
 	@Override
-	protected AbstractIntegrationMessageBuilder<String> doTransform(javax.mail.Message mailMessage) throws Exception {
+	protected AbstractIntegrationMessageBuilder<String> doTransform(javax.mail.Message mailMessage)
+			throws Exception { // NOSONAR
+
 		Object content = mailMessage.getContent();
 		if (content instanceof String) {
 			return this.getMessageBuilderFactory().withPayload((String) content);