JAMES-1877 Extract delays and max retries calculations from RemoteDelivery
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/8a8af97a Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/8a8af97a Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/8a8af97a Branch: refs/heads/master Commit: 8a8af97a5f569fb01f24b72bfbb8e481e4c26f26 Parents: 1193b6b Author: Benoit Tellier <[email protected]> Authored: Tue Nov 29 12:02:07 2016 +0700 Committer: Benoit Tellier <[email protected]> Committed: Tue Jan 10 14:35:32 2017 +0700 ---------------------------------------------------------------------- .../james/transport/mailets/RemoteDelivery.java | 107 +----------- .../remoteDelivery/DelaysAndMaxRetry.java | 166 +++++++++++++++++++ .../mailets/remoteDelivery/DelayTest.java | 21 +++ .../remoteDelivery/DelaysAndMaxRetryTest.java | 136 +++++++++++++++ 4 files changed, 329 insertions(+), 101 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/8a8af97a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java index b23f79d..9ffd164 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java @@ -64,6 +64,7 @@ import org.apache.james.queue.api.MailQueue.MailQueueException; import org.apache.james.queue.api.MailQueue.MailQueueItem; import org.apache.james.queue.api.MailQueueFactory; import org.apache.james.transport.mailets.remoteDelivery.Delay; +import org.apache.james.transport.mailets.remoteDelivery.DelaysAndMaxRetry; import org.apache.james.transport.mailets.remoteDelivery.HeloNameProvider; import org.apache.james.transport.mailets.remoteDelivery.RemoteDeliverySocketFactory; import org.apache.mailet.HostAddress; @@ -170,7 +171,7 @@ public class RemoteDelivery extends GenericMailet implements Runnable { /** * Maximum no. of retries (Defaults to 5). */ - private int maxRetries = 5; + private int maxRetries; /** * Default number of ms to timeout on smtp delivery @@ -264,61 +265,11 @@ public class RemoteDelivery extends GenericMailet implements Runnable { logger = getMailetContext().getLogger(); - // Create list of Delay Times. - ArrayList<Delay> delayTimesList = new ArrayList<Delay>(); try { - if (getInitParameter("delayTime") != null) { - - // parses delayTimes specified in config file. - String delayTimesParm = getInitParameter("delayTime"); - - // Split on commas - StringTokenizer st = new StringTokenizer(delayTimesParm, ","); - while (st.hasMoreTokens()) { - String delayTime = st.nextToken(); - delayTimesList.add(new Delay(delayTime)); - } - } else { - // Use default delayTime. - delayTimesList.add(new Delay()); - } - } catch (Exception e) { - log("Invalid delayTime setting: " + getInitParameter("delayTime")); - } - - try { - // Get No. of Max Retries. - if (getInitParameter("maxRetries") != null) { - maxRetries = Integer.parseInt(getInitParameter("maxRetries")); - } - - // Check consistency of 'maxRetries' with delayTimesList attempts. - int totalAttempts = calcTotalAttempts(delayTimesList); - - // If inconsistency found, fix it. - if (totalAttempts > maxRetries) { - log("Total number of delayTime attempts exceeds maxRetries specified. " + " Increasing maxRetries from " + maxRetries + " to " + totalAttempts); - maxRetries = totalAttempts; - } else { - int extra = maxRetries - totalAttempts; - if (extra != 0) { - log("maxRetries is larger than total number of attempts specified. " + "Increasing last delayTime with " + extra + " attempts "); - - // Add extra attempts to the last delayTime. - if (delayTimesList.size() != 0) { - // Get the last delayTime. - Delay delay = delayTimesList.get(delayTimesList.size() - 1); - - // Increase no. of attempts. - delay.setAttempts(delay.getAttempts() + extra); - log("Delay of " + delay.getDelayTime() + " msecs is now attempted: " + delay.getAttempts() + " times"); - } else { - throw new MessagingException("No delaytimes, cannot continue"); - } - } - } - delayTimes = expandDelays(delayTimesList); - + int intendedMaxRetries = Integer.parseInt(getInitParameter("maxRetries", "5")); + DelaysAndMaxRetry delaysAndMaxRetry = DelaysAndMaxRetry.from(intendedMaxRetries, getInitParameter("delayTime")); + maxRetries = delaysAndMaxRetry.getMaxRetries(); + delayTimes = delaysAndMaxRetry.getExpendedDelays(); } catch (Exception e) { log("Invalid maxRetries setting: " + getInitParameter("maxRetries")); } @@ -435,52 +386,6 @@ public class RemoteDelivery extends GenericMailet implements Runnable { } /** - * Calculates Total no. of attempts for the specified delayList. - * - * @param delayList list of 'Delay' objects - * @return total no. of retry attempts - */ - private int calcTotalAttempts(ArrayList<Delay> delayList) { - int sum = 0; - for (Delay delay : delayList) { - sum += delay.getAttempts(); - } - return sum; - } - - /** - * <p> - * This method expands an ArrayList containing Delay objects into an array - * holding the only delaytime in the order. - * </p> - * <p/> - * So if the list has 2 Delay objects the first having attempts=2 and - * delaytime 4000 the second having attempts=1 and delaytime=300000 will be - * expanded into this array: - * <p/> - * <pre> - * long[0] = 4000 - * long[1] = 4000 - * long[2] = 300000 - * </pre> - * - * @param list the list to expand - * @return the expanded list - */ - private long[] expandDelays(ArrayList<Delay> list) { - long[] delays = new long[calcTotalAttempts(list)]; - Iterator<Delay> i = list.iterator(); - int idx = 0; - while (i.hasNext()) { - Delay delay = i.next(); - for (int j = 0; j < delay.getAttempts(); j++) { - delays[idx++] = delay.getDelayTime(); - } - } - return delays; - } - - /** * This method returns, given a retry-count, the next delay time to use. * * @param retry_count the current retry_count. http://git-wip-us.apache.org/repos/asf/james-project/blob/8a8af97a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetry.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetry.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetry.java new file mode 100644 index 0000000..10b6cca --- /dev/null +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetry.java @@ -0,0 +1,166 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.transport.mailets.remoteDelivery; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.StringTokenizer; + +import javax.mail.MessagingException; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; + +public class DelaysAndMaxRetry { + + private static final Logger LOGGER = LoggerFactory.getLogger(DelaysAndMaxRetry.class); + + public static DelaysAndMaxRetry from(int intendedMaxRetries, String delaysAsString) throws MessagingException { + // Create list of Delay Times. + ArrayList<Delay> delayTimesList = createDelayList(delaysAsString); + + // Check consistency of 'maxRetries' with delayTimesList attempts. + int totalAttempts = calcTotalAttempts(delayTimesList); + + // If inconsistency found, fix it. + if (totalAttempts > intendedMaxRetries) { + LOGGER.warn("Total number of delayTime attempts exceeds maxRetries specified. " + " Increasing maxRetries from " + intendedMaxRetries + " to " + totalAttempts); + return new DelaysAndMaxRetry(totalAttempts, delayTimesList); + } else { + int extra = intendedMaxRetries - totalAttempts; + if (extra != 0) { + LOGGER.warn("maxRetries is larger than total number of attempts specified. " + "Increasing last delayTime with " + extra + " attempts "); + + // Add extra attempts to the last delayTime. + if (delayTimesList.size() != 0) { + // Get the last delayTime. + Delay delay = delayTimesList.get(delayTimesList.size() - 1); + + // Increase no. of attempts. + delay.setAttempts(delay.getAttempts() + extra); + LOGGER.warn("Delay of " + delay.getDelayTime() + " msecs is now attempted: " + delay.getAttempts() + " times"); + } else { + throw new MessagingException("No delaytimes, cannot continue"); + } + } + return new DelaysAndMaxRetry(intendedMaxRetries, delayTimesList); + } + } + + private static ArrayList<Delay> createDelayList(String delaysAsString) { + ArrayList<Delay> delayTimesList = new ArrayList<Delay>(); + try { + if (delaysAsString != null) { + + // Split on commas + StringTokenizer st = new StringTokenizer(delaysAsString, ","); + while (st.hasMoreTokens()) { + String delayTime = st.nextToken(); + delayTimesList.add(new Delay(delayTime)); + } + } else { + // Use default delayTime. + delayTimesList.add(new Delay()); + } + } catch (Exception e) { + LOGGER.warn("Invalid delayTime setting: " + delaysAsString); + } + return delayTimesList; + } + + /** + * Calculates Total no. of attempts for the specified delayList. + * + * @param delayList list of 'Delay' objects + * @return total no. of retry attempts + */ + private static int calcTotalAttempts(List<Delay> delayList) { + int sum = 0; + for (Delay delay : delayList) { + sum += delay.getAttempts(); + } + return sum; + } + + private final int maxRetries; + private final List<Delay> delays; + + @VisibleForTesting + DelaysAndMaxRetry(int maxRetries, List<Delay> delays) { + this.maxRetries = maxRetries; + this.delays = ImmutableList.copyOf(delays); + } + + public int getMaxRetries() { + return maxRetries; + } + + /** + * <p> + * This method expands an ArrayList containing Delay objects into an array + * holding the only delaytime in the order. + * </p> + * <p/> + * So if the list has 2 Delay objects the first having attempts=2 and + * delaytime 4000 the second having attempts=1 and delaytime=300000 will be + * expanded into this array: + * <p/> + * <pre> + * long[0] = 4000 + * long[1] = 4000 + * long[2] = 300000 + * </pre> + * + * @param list the list to expand + * @return the expanded list + */ + public long[] getExpendedDelays() { + long[] delaysAsLong = new long[calcTotalAttempts(delays)]; + Iterator<Delay> i = delays.iterator(); + int idx = 0; + while (i.hasNext()) { + Delay delay = i.next(); + for (int j = 0; j < delay.getAttempts(); j++) { + delaysAsLong[idx++] = delay.getDelayTime(); + } + } + return delaysAsLong; + } + + @Override + public boolean equals(Object o) { + if (o instanceof DelaysAndMaxRetry) { + DelaysAndMaxRetry that = (DelaysAndMaxRetry) o; + return Objects.equal(this.maxRetries, that.maxRetries) + && Objects.equal(this.delays, that.delays); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hashCode(maxRetries, delays); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/8a8af97a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelayTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelayTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelayTest.java index 92ce97b..f23c918 100644 --- a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelayTest.java +++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelayTest.java @@ -76,6 +76,20 @@ public class DelayTest { } @Test + public void stringConstructorShouldThrowWhenAttemptsOmitted() throws Exception { + expectedException.expect(NumberFormatException.class); + + new Delay("*36"); + } + + @Test + public void stringConstructorShouldThrowWhenDelayOmitted() throws Exception { + expectedException.expect(MessagingException.class); + + new Delay("2*"); + } + + @Test public void stringConstructorShouldWorkForNumberAttemptsAndUnit() throws Exception { assertThat(new Delay("2*36s")).isEqualTo(new Delay(2, 36000)); } @@ -93,4 +107,11 @@ public class DelayTest { new Delay("36invalid"); } + + @Test + public void stringConstructorShouldThrowOnEmptyString() throws Exception { + expectedException.expect(MessagingException.class); + + new Delay(""); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/8a8af97a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetryTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetryTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetryTest.java new file mode 100644 index 0000000..da4ecda --- /dev/null +++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/DelaysAndMaxRetryTest.java @@ -0,0 +1,136 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.transport.mailets.remoteDelivery; + +import static org.assertj.core.api.Assertions.assertThat; + +import javax.mail.MessagingException; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import com.google.common.collect.ImmutableList; + +public class DelaysAndMaxRetryTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void fromShouldParseSingleDelay() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(1, "1s"); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(1, ImmutableList.of(new Delay(1, 1000))); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldParseTwoDelays() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(2, "1s,2s"); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(2, ImmutableList.of(new Delay(1, 1000), new Delay(1, 2000))); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldAdaptMaxRetriesWhenUnderAttempts() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(1, "1s,2*2s"); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(3, ImmutableList.of(new Delay(1, 1000), new Delay(2, 2000))); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldAdaptDelaysWhenUnderMaxRetries() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(4, "1s,2*2s"); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(4, ImmutableList.of(new Delay(1, 1000), new Delay(3, 2000))); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldHandleNullValuesForDelayAsString() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(1, null); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(1, ImmutableList.of(new Delay(Delay.DEFAULT_ATTEMPTS, Delay.DEFAULT_DELAY_TIME))); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldIgnoreEmptyDelay() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(1, "1s,,2s"); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(2, ImmutableList.of(new Delay(1, 1000), new Delay(1, 2000))); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldHandleParsingFailures() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(3, "1s,invalid,2s"); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(3, ImmutableList.of(new Delay(3, 1000))); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldHandleEmptyStringWithZeroMaxRetries() throws Exception { + DelaysAndMaxRetry actual = DelaysAndMaxRetry.from(0, ""); + + DelaysAndMaxRetry expected = new DelaysAndMaxRetry(0, ImmutableList.<Delay>of()); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void fromShouldThrowOnEmptyStringWithNonZeroMaxRetry() throws Exception { + expectedException.expect(MessagingException.class); + + DelaysAndMaxRetry.from(2, ""); + } + + @Test + public void getExpendedDelaysShouldReturnEmptyWhenNoDelay() throws Exception { + DelaysAndMaxRetry testee = DelaysAndMaxRetry.from(0, ""); + + assertThat(testee.getExpendedDelays()).isEmpty(); + } + + @Test + public void getExpendedDelaysShouldExpandSingleDelays() throws Exception { + DelaysAndMaxRetry testee = DelaysAndMaxRetry.from(3, "1*1S,1*2S,1*5S"); + + assertThat(testee.getExpendedDelays()).containsExactly(1000, 2000, 5000); + } + + @Test + public void getExpendedDelaysShouldExpandMultipleDelays() throws Exception { + DelaysAndMaxRetry testee = DelaysAndMaxRetry.from(3, "1*1S,2*2S,2*5S"); + + assertThat(testee.getExpendedDelays()).containsExactly(1000, 2000, 2000, 5000, 5000); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
