From 119db815c21e659f2099cdc6a54e9e3048ffb5ec Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 22 Jan 2019 12:51:29 -0500 Subject: [PATCH] * 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 --- .travis.yml | 6 - .../mail/AbstractMailReceiver.java | 111 ++++++++++-------- .../mail/ImapMailReceiverTests.java | 38 +++--- 3 files changed, 79 insertions(+), 76 deletions(-) diff --git a/.travis.yml b/.travis.yml index 93a98e3b40..815c5397e5 100644 --- a/.travis.yml +++ b/.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: diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java index 2a7cb45433..7caf19a8b2 100755 --- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java +++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java @@ -341,66 +341,77 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl @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 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 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 headers) { @@ -525,7 +536,6 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl * 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 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl /** * 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 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl /** * 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 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl * 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 * + * @author Gary Russell + * + * @since 2.2 */ private final class IntegrationMimeMessage extends MimeMessage { @@ -625,7 +634,7 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl @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); diff --git a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java b/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java index 30cc02b13d..e0b135fe96 100644 --- a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java +++ b/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.FlagTerm; 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 @@ import com.sun.mail.imap.IMAPFolder; @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,15 +156,15 @@ public class ImapMailReceiverTests { @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); } @@ -172,16 +172,16 @@ public class ImapMailReceiverTests { @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 class ImapMailReceiverTests { } 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 class ImapMailReceiverTests { adapter.stop(); taskScheduler.shutdown(); - assertTrue(imapIdleServer.assertReceived("storeUserFlag")); + assertTrue(this.imapIdleServer.assertReceived("storeUserFlag")); } @Test