JAMES-2220 cover MailImpl name generation with tests
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/c80bc056 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/c80bc056 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/c80bc056 Branch: refs/heads/master Commit: c80bc05615e2917a72c0b62c49ffb090ed218f3c Parents: 007ca71 Author: Matthieu Baechler <matth...@apache.org> Authored: Tue Nov 14 16:49:26 2017 +0100 Committer: Antoine Duprat <adup...@linagora.com> Committed: Thu Nov 16 12:30:31 2017 +0100 ---------------------------------------------------------------------- .../org/apache/james/server/core/MailImpl.java | 79 ++++++++++---------- .../apache/james/server/core/MailImplTest.java | 50 +++++++++++++ 2 files changed, 89 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/c80bc056/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java b/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java index fb0bf26..4ba435c 100644 --- a/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java +++ b/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java @@ -28,7 +28,6 @@ import java.io.ObjectOutputStream; import java.io.OptionalDataException; import java.io.OutputStream; import java.io.Serializable; -import java.util.ArrayList; import java.util.Collection; import java.util.Date; import java.util.HashMap; @@ -80,9 +79,47 @@ public class MailImpl implements Disposable, Mail { * @throws MessagingException when the message is not clonable */ public static MailImpl duplicate(Mail mail) throws MessagingException { - return new MailImpl(mail, newName(mail)); + return new MailImpl(mail, deriveNewName(mail.getName())); } + private static final java.util.Random random = new java.util.Random(); // Used + // to + // generate + // new + // mail + // names + + /** + * Create a unique new primary key name for the given MailObject. + * + * @param currentName the mail to use as the basis for the new mail name + * @return a new name + */ + @VisibleForTesting static String deriveNewName(String currentName) throws MessagingException { + + // Checking if the original mail name is too long, perhaps because of a + // loop caused by a configuration error. + // it could cause a "null pointer exception" in AvalonMailRepository + // much + // harder to understand. + if (currentName.length() > 76) { + int count = 0; + int index = 0; + while ((index = currentName.indexOf('!', index + 1)) >= 0) { + count++; + } + // It looks like a configuration loop. It's better to stop. + if (count > 7) { + throw new MessagingException("Unable to create a new message name: too long." + " Possible loop in config.xml."); + } else { + currentName = currentName.substring(0, 76); + } + } + + return currentName + "-!" + random.nextInt(1048576); + } + + private static final Logger LOGGER = LoggerFactory.getLogger(MailImpl.class); /** @@ -532,44 +569,6 @@ public class MailImpl implements Disposable, Mail { return in.readObject(); } - private static final java.util.Random random = new java.util.Random(); // Used - // to - // generate - // new - // mail - // names - - /** - * Create a unique new primary key name for the given MailObject. - * - * @param mail the mail to use as the basis for the new mail name - * @return a new name - */ - public static String newName(Mail mail) throws MessagingException { - String oldName = mail.getName(); - - // Checking if the original mail name is too long, perhaps because of a - // loop caused by a configuration error. - // it could cause a "null pointer exception" in AvalonMailRepository - // much - // harder to understand. - if (oldName.length() > 76) { - int count = 0; - int index = 0; - while ((index = oldName.indexOf('!', index + 1)) >= 0) { - count++; - } - // It looks like a configuration loop. It's better to stop. - if (count > 7) { - throw new MessagingException("Unable to create a new message name: too long." + " Possible loop in config.xml."); - } else { - oldName = oldName.substring(0, 76); - } - } - - return oldName + "-!" + random.nextInt(1048576); - } - /** * Generate a new identifier/name for a mail being processed by this server. * http://git-wip-us.apache.org/repos/asf/james-project/blob/c80bc056/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java b/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java index f1beea2..e90e4d0 100644 --- a/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java +++ b/server/container/core/src/test/java/org/apache/james/server/core/MailImplTest.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Date; import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; import javax.mail.MessagingException; import javax.mail.Session; @@ -35,8 +36,10 @@ import org.apache.james.core.MailAddress; import org.apache.mailet.Mail; import org.apache.mailet.base.test.MailUtil; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; +import com.github.fge.lambdas.Throwing; import com.google.common.collect.ImmutableList; public class MailImplTest { @@ -163,4 +166,51 @@ public class MailImplTest { .isInstanceOf(NullPointerException.class); } + @Test + public void deriveNewNameShouldThrowOnNull() { + assertThatThrownBy(() -> MailImpl.deriveNewName(null)).isInstanceOf(NullPointerException.class); + } + + @Test + public void deriveNewNameShouldGenerateNonEmptyStringOnEmpty() throws MessagingException { + assertThat(MailImpl.deriveNewName("")).isNotEmpty(); + } + + @Test + public void deriveNewNameShouldNeverGenerateMoreThan86Characters() throws MessagingException { + String longString = "mu1Eeseemu1Eeseemu1Eeseemu1Eeseemu1Eeseemu1Eeseemu1Eeseemu1Eeseemu1Eeseeseemu1Eesee"; + assertThat(MailImpl.deriveNewName(longString).length()).isLessThan(86); + } + + @Test + public void deriveNewNameShouldThrowWhenMoreThan8NestedCalls() throws MessagingException { + String called6Times = IntStream.range(0, 8) + .mapToObj(String::valueOf) + .reduce("average value ", Throwing.binaryOperator((left, right) -> MailImpl.deriveNewName(left))); + assertThatThrownBy(() -> MailImpl.deriveNewName(called6Times)).isInstanceOf(MessagingException.class); + } + + @Test + @Ignore("short value doesn't trigger the assertion") + public void deriveNewNameShouldThrowWhenMoreThan8NestedCallsAndSmallInitialValue() throws MessagingException { + String called6Times = IntStream.range(0, 8) + .mapToObj(String::valueOf) + .reduce("small", Throwing.binaryOperator((left, right) -> MailImpl.deriveNewName(left))); + assertThatThrownBy(() -> MailImpl.deriveNewName(called6Times)).isInstanceOf(MessagingException.class); + } + + @Test + @Ignore("truncation make impossible to detect 8 calls") + public void deriveNewNameShouldThrowWhenMoreThan8NestedCallsAndLongInitialValue() throws MessagingException { + String called6Times = IntStream.range(0, 8) + .mapToObj(String::valueOf) + .reduce("looooooonnnnnngggggggggggggggg", Throwing.binaryOperator((left, right) -> MailImpl.deriveNewName(left))); + assertThatThrownBy(() -> MailImpl.deriveNewName(called6Times)).isInstanceOf(MessagingException.class); + } + + + @Test + public void deriveNewNameShouldGenerateNotEqualsCurrentName() throws MessagingException { + assertThat(MailImpl.deriveNewName("current")).isNotEqualTo("current"); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
