* Fix new Sonar issue in the AbstractMailReceiver (#2709)

* * Fix new Sonar issue in the `AbstractMailReceiver`

* * Move `unlock()` to its own `finally`

* * Make `ImapMailReceiverTests.imapIdleServer` non-static to allow
recreate the mail server for each test method

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

* * Remove MongoDb download from Travis config

* * Fix Checkstyle violation

Author: artembilan

.travis.yml

@@ -5,12 +5,6 @@ services:
   - mongodb
   - rabbitmq
   - redis-server
-addons:
-  apt:
-    sources:
-    - mongodb-3.0-precise
-    packages:
-    - mongodb-org-server
 before_cache:
   - rm -f $HOME/.gradle/caches/modules-2/modules-2.lock
 cache:

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

@@ -341,66 +341,77 @@ private Folder obtainFolderInstance() throws MessagingException {
 	@Override
 	public Object[] receive() throws javax.mail.MessagingException {
 		this.folderReadLock.lock();
+		try {
+			try {
+				obtainFolder();
+				MimeMessage[] filteredMessages = searchAndFilterMessages();
+				if (this.headerMapper != null) {
+					org.springframework.messaging.Message<?>[] converted =
+							new org.springframework.messaging.Message<?>[filteredMessages.length];
+					int n = 0;
+					for (MimeMessage message : filteredMessages) {
+						Map<String, Object> headers = this.headerMapper.toHeaders(message);
+						converted[n++] =
+								getMessageBuilderFactory()
+										.withPayload(extractContent(message, headers))
+										.copyHeaders(headers)
+										.build();
+					}
+					return converted;
+				}
+				else {
+					return filteredMessages;
+				}
+			}
+			finally {
+				MailTransportUtils.closeFolder(this.folder, this.shouldDeleteMessages);
+			}
+		}
+		finally {
+			this.folderReadLock.unlock();
+		}
+	}
+
+	private void obtainFolder() throws MessagingException {
 		Folder folder = getFolder();
 		if (folder == null || !folder.isOpen()) {
 			this.folderReadLock.unlock();
 			this.folderWriteLock.lock();
 			try {
 				openFolder();
-				folder = getFolder();
 				this.folderReadLock.lock();
 			}
 			finally {
 				this.folderWriteLock.unlock();
 			}
 		}
-		try {
-			if (this.logger.isInfoEnabled()) {
-				this.logger.info("attempting to receive mail from folder [" + folder.getFullName() + "]");
-			}
-			Message[] messages = searchForNewMessages();
-			if (this.maxFetchSize > 0 && messages.length > this.maxFetchSize) {
-				Message[] reducedMessages = new Message[this.maxFetchSize];
-				System.arraycopy(messages, 0, reducedMessages, 0, this.maxFetchSize);
-				messages = reducedMessages;
-			}
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("found " + messages.length + " new messages");
-			}
-			if (messages.length > 0) {
-				fetchMessages(messages);
-			}
-
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Received " + messages.length + " messages");
-			}
+	}
 
-			MimeMessage[] filteredMessages = filterMessagesThruSelector(messages);
-
-			postProcessFilteredMessages(filteredMessages);
-
-			if (this.headerMapper != null) {
-				org.springframework.messaging.Message<?>[] converted =
-						new org.springframework.messaging.Message<?>[filteredMessages.length];
-				int n = 0;
-				for (MimeMessage message : filteredMessages) {
-					Map<String, Object> headers = this.headerMapper.toHeaders(message);
-					converted[n++] =
-							getMessageBuilderFactory()
-									.withPayload(extractContent(message, headers))
-									.copyHeaders(headers)
-									.build();
-				}
-				return converted;
-			}
-			else {
-				return filteredMessages;
-			}
+	private MimeMessage[] searchAndFilterMessages() throws MessagingException {
+		if (this.logger.isInfoEnabled()) {
+			this.logger.info("attempting to receive mail from folder [" + this.folder.getFullName() + "]");
 		}
-		finally {
-			MailTransportUtils.closeFolder(folder, this.shouldDeleteMessages);
-			this.folderReadLock.unlock();
+		Message[] messages = searchForNewMessages();
+		if (this.maxFetchSize > 0 && messages.length > this.maxFetchSize) {
+			Message[] reducedMessages = new Message[this.maxFetchSize];
+			System.arraycopy(messages, 0, reducedMessages, 0, this.maxFetchSize);
+			messages = reducedMessages;
+		}
+		if (this.logger.isDebugEnabled()) {
+			this.logger.debug("found " + messages.length + " new messages");
+		}
+		if (messages.length > 0) {
+			fetchMessages(messages);
 		}
+
+		if (this.logger.isDebugEnabled()) {
+			this.logger.debug("Received " + messages.length + " messages");
+		}
+
+		MimeMessage[] filteredMessages = filterMessagesThruSelector(messages);
+
+		postProcessFilteredMessages(filteredMessages);
+		return filteredMessages;
 	}
 
 	private Object extractContent(MimeMessage message, Map<String, Object> headers) {
@@ -525,7 +536,6 @@ private MimeMessage[] filterMessagesThruSelector(Message[] messages) throws Mess
 	 * Fetches the specified messages from this receiver's folder. Default
 	 * implementation {@link Folder#fetch(Message[], FetchProfile) fetches}
 	 * every {@link javax.mail.FetchProfile.Item}.
-	 *
 	 * @param messages the messages to fetch
 	 * @throws MessagingException in case of JavaMail errors
 	 */
@@ -539,7 +549,6 @@ protected void fetchMessages(Message[] messages) throws MessagingException {
 
 	/**
 	 * Deletes the given messages from this receiver's folder.
-	 *
 	 * @param messages the messages to delete
 	 * @throws MessagingException in case of JavaMail errors
 	 */
@@ -552,7 +561,6 @@ protected void deleteMessages(Message[] messages) throws MessagingException {
 	/**
 	 * Optional method allowing you to set additional flags.
 	 * Currently only implemented in IMapMailReceiver.
-	 *
 	 * @param message The message.
 	 * @throws MessagingException A MessagingException.
 	 */
@@ -593,9 +601,10 @@ Store getStore() {
 	 * Since we copy the message to eagerly fetch the message, it has no folder.
 	 * However, we need to make a folder available in case the user wants to
 	 * perform operations on the message in the folder later in the flow.
+	 *
 	 * @author Gary Russell
-	 * @since 2.2
 	 *
+	 * @since 2.2
 	 */
 	private final class IntegrationMimeMessage extends MimeMessage {
 
@@ -625,7 +634,7 @@ private final class IntegrationMimeMessage extends MimeMessage {
 		@Override
 		public Folder getFolder() {
 			try {
-				return AbstractMailReceiver.this.obtainFolderInstance();
+				return obtainFolderInstance();
 			}
 			catch (MessagingException e) {
 				throw new org.springframework.messaging.MessagingException("Unable to obtain the mail folder", e);

spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.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.
@@ -66,8 +66,8 @@
 import javax.mail.search.FromTerm;
 
 import org.apache.commons.logging.Log;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
@@ -112,7 +112,7 @@
 @DirtiesContext
 public class ImapMailReceiverTests {
 
-	private static final ImapServer imapIdleServer = TestMailServer.imap(0);
+	private final ImapServer imapIdleServer = TestMailServer.imap(0);
 
 	@Rule
 	public final LongRunningIntegrationTest longRunningIntegrationTest = new LongRunningIntegrationTest();
@@ -123,24 +123,24 @@ public class ImapMailReceiverTests {
 	@Autowired
 	private ApplicationContext context;
 
-	@BeforeClass
-	public static void setup() throws InterruptedException {
+	@Before
+	public void setup() throws InterruptedException {
 		int n = 0;
-		while (n++ < 100 && (!imapIdleServer.isListening())) {
+		while (n++ < 100 && (!this.imapIdleServer.isListening())) {
 			Thread.sleep(100);
 		}
 		assertTrue(n < 100);
 	}
 
-	@AfterClass
-	public static void tearDown() {
-		imapIdleServer.stop();
+	@After
+	public void tearDown() {
+		this.imapIdleServer.stop();
 	}
 
 	@Test
 	public void testIdleWithServerCustomSearch() throws Exception {
 		ImapMailReceiver receiver =
-				new ImapMailReceiver("imap://user:pw@localhost:" + imapIdleServer.getPort() + "/INBOX");
+				new ImapMailReceiver("imap://user:pw@localhost:" + this.imapIdleServer.getPort() + "/INBOX");
 		receiver.setSearchTermStrategy((supportedFlags, folder) -> {
 			try {
 				FromTerm fromTerm = new FromTerm(new InternetAddress("bar@baz"));
@@ -156,32 +156,32 @@ public void testIdleWithServerCustomSearch() throws Exception {
 	@Test
 	public void testIdleWithServerDefaultSearch() throws Exception {
 		ImapMailReceiver receiver =
-				new ImapMailReceiver("imap://user:pw@localhost:" + imapIdleServer.getPort() + "/INBOX");
+				new ImapMailReceiver("imap://user:pw@localhost:" + this.imapIdleServer.getPort() + "/INBOX");
 		testIdleWithServerGuts(receiver, false);
-		assertTrue(imapIdleServer.assertReceived("searchWithUserFlag"));
+		assertTrue(this.imapIdleServer.assertReceived("searchWithUserFlag"));
 	}
 
 	@Test
 	public void testIdleWithMessageMapping() throws Exception {
 		ImapMailReceiver receiver =
-				new ImapMailReceiver("imap://user:pw@localhost:" + imapIdleServer.getPort() + "/INBOX");
+				new ImapMailReceiver("imap://user:pw@localhost:" + this.imapIdleServer.getPort() + "/INBOX");
 		receiver.setHeaderMapper(new DefaultMailHeaderMapper());
 		testIdleWithServerGuts(receiver, true);
 	}
 
 	@Test
 	public void testIdleWithServerDefaultSearchSimple() throws Exception {
 		ImapMailReceiver receiver =
-				new ImapMailReceiver("imap://user:pw@localhost:" + imapIdleServer.getPort() + "/INBOX");
+				new ImapMailReceiver("imap://user:pw@localhost:" + this.imapIdleServer.getPort() + "/INBOX");
 		receiver.setSimpleContent(true);
 		testIdleWithServerGuts(receiver, false, true);
-		assertTrue(imapIdleServer.assertReceived("searchWithUserFlag"));
+		assertTrue(this.imapIdleServer.assertReceived("searchWithUserFlag"));
 	}
 
 	@Test
 	public void testIdleWithMessageMappingSimple() throws Exception {
 		ImapMailReceiver receiver =
-				new ImapMailReceiver("imap://user:pw@localhost:" + imapIdleServer.getPort() + "/INBOX");
+				new ImapMailReceiver("imap://user:pw@localhost:" + this.imapIdleServer.getPort() + "/INBOX");
 		receiver.setSimpleContent(true);
 		receiver.setHeaderMapper(new DefaultMailHeaderMapper());
 		testIdleWithServerGuts(receiver, true, true);
@@ -192,7 +192,7 @@ public void testIdleWithServerGuts(ImapMailReceiver receiver, boolean mapped) th
 	}
 
 	public void testIdleWithServerGuts(ImapMailReceiver receiver, boolean mapped, boolean simple) throws Exception {
-		imapIdleServer.resetServer();
+		this.imapIdleServer.resetServer();
 		Properties mailProps = new Properties();
 		mailProps.put("mail.debug", "true");
 		mailProps.put("mail.imap.connectionpool.debug", "true");
@@ -256,7 +256,7 @@ public void testIdleWithServerGuts(ImapMailReceiver receiver, boolean mapped, bo
 
 		adapter.stop();
 		taskScheduler.shutdown();
-		assertTrue(imapIdleServer.assertReceived("storeUserFlag"));
+		assertTrue(this.imapIdleServer.assertReceived("storeUserFlag"));
 	}
 
 	@Test