JAMES-2366 Mappings should be sorted when building the list
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/048e3625 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/048e3625 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/048e3625 Branch: refs/heads/master Commit: 048e362529ba5605c9551dc63ac14611ab4a5ee2 Parents: 71a465e Author: Antoine Duprat <adup...@linagora.com> Authored: Thu Mar 29 15:28:16 2018 +0200 Committer: Antoine Duprat <adup...@linagora.com> Committed: Fri Apr 6 14:39:17 2018 +0200 ---------------------------------------------------------------------- .../rrt/lib/AbstractRecipientRewriteTable.java | 16 +------ .../org/apache/james/rrt/lib/MappingsImpl.java | 18 ++++++- .../lib/AbstractRecipientRewriteTableTest.java | 36 -------------- .../apache/james/rrt/lib/MappingsImplTest.java | 49 +++++++++++++------- 4 files changed, 50 insertions(+), 69 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/048e3625/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java ---------------------------------------------------------------------- diff --git a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java index 2b11cee..8b314b1 100644 --- a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java +++ b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTable.java @@ -40,8 +40,6 @@ import org.apache.james.rrt.lib.Mapping.Type; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.annotations.VisibleForTesting; - /** * */ @@ -423,24 +421,12 @@ public abstract class AbstractRecipientRewriteTable implements RecipientRewriteT String mappings = mapAddressInternal(user, domain); if (mappings != null) { - return sortMappings(MappingsImpl.fromRawString(mappings)); + return MappingsImpl.fromRawString(mappings); } else { return null; } } - @VisibleForTesting static Mappings sortMappings(Mappings mappings) { - if (mappings.contains(Mapping.Type.Domain)) { - return - MappingsImpl.builder() - .addAll(mappings.select(Mapping.Type.Domain)) - .addAll(mappings.exclude(Mapping.Type.Domain)) - .build(); - } else { - return mappings; - } - } - private void checkMapping(String user, Domain domain, Mapping mapping) throws RecipientRewriteTableException { Mappings mappings = getUserDomainMappings(user, domain); if (mappings != null && mappings.contains(mapping)) { http://git-wip-us.apache.org/repos/asf/james-project/blob/048e3625/server/data/data-library/src/main/java/org/apache/james/rrt/lib/MappingsImpl.java ---------------------------------------------------------------------- diff --git a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/MappingsImpl.java b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/MappingsImpl.java index 73949f6..eab5fd8 100644 --- a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/MappingsImpl.java +++ b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/MappingsImpl.java @@ -23,6 +23,7 @@ package org.apache.james.rrt.lib; import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.Iterator; import java.util.Optional; import java.util.StringTokenizer; @@ -110,8 +111,23 @@ public class MappingsImpl implements Mappings, Serializable { } public MappingsImpl build() { - return new MappingsImpl(mappings.build()); + Comparator<? super Mapping> byTypeComparator = (mapping1, mapping2) -> { + if (mapping1.getType() == mapping2.getType()) { + return mapping1.asString().compareTo(mapping2.asString()); + } + if (mapping1.getType() == Type.Domain) { + return -1; + } + if (mapping2.getType() == Type.Domain) { + return 1; + } + return mapping1.asString().compareTo(mapping2.asString()); + }; + return new MappingsImpl(mappings.build().stream() + .sorted(byTypeComparator) + .collect(Guavate.toImmutableList())); } + } http://git-wip-us.apache.org/repos/asf/james-project/blob/048e3625/server/data/data-library/src/test/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTableTest.java ---------------------------------------------------------------------- diff --git a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTableTest.java b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTableTest.java index 26b3187..3c69fab 100644 --- a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTableTest.java +++ b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/AbstractRecipientRewriteTableTest.java @@ -231,42 +231,6 @@ public abstract class AbstractRecipientRewriteTableTest { } @Test - public void sortMappingsShouldReturnEmptyWhenEmpty() { - assertThat(AbstractRecipientRewriteTable.sortMappings(MappingsImpl.empty())).isEmpty(); - } - - @Test - public void sortMappingsShouldReturnSameStringWhenSingleDomainAlias() { - String singleDomainAlias = Type.Domain.asPrefix() + "first"; - assertThat(AbstractRecipientRewriteTable.sortMappings(MappingsImpl.fromRawString(singleDomainAlias))) - .containsExactly(MappingImpl.domain(Domain.of("first"))); - } - - @Test - public void sortMappingsShouldReturnSameStringWhenTwoDomainAliases() { - MappingsImpl mappings = MappingsImpl.builder() - .add(Type.Domain.asPrefix() + "first") - .add(Type.Domain.asPrefix() + "second") - .build(); - assertThat(AbstractRecipientRewriteTable.sortMappings(mappings)).isEqualTo(mappings); - } - - @Test - public void sortMappingsShouldPutDomainAliasFirstWhenVariousMappings() { - String regexMapping = Type.Regex.asPrefix() + "first"; - String domainMapping = Type.Domain.asPrefix() + "second"; - MappingsImpl mappings = MappingsImpl.builder() - .add(regexMapping) - .add(domainMapping) - .build(); - assertThat(AbstractRecipientRewriteTable.sortMappings(mappings)) - .isEqualTo(MappingsImpl.builder() - .add(domainMapping) - .add(regexMapping) - .build()); - } - - @Test public void addMappingShouldThrowWhenMappingAlreadyExists() throws Exception { String user = "test"; Domain domain = Domain.LOCALHOST; http://git-wip-us.apache.org/repos/asf/james-project/blob/048e3625/server/data/data-library/src/test/java/org/apache/james/rrt/lib/MappingsImplTest.java ---------------------------------------------------------------------- diff --git a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/MappingsImplTest.java b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/MappingsImplTest.java index 496c74e..efb5add 100644 --- a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/MappingsImplTest.java +++ b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/MappingsImplTest.java @@ -45,82 +45,82 @@ public class MappingsImplTest { @Test public void fromRawStringShouldReturnSingletonCollectionWhenSingleElementString() { MappingsImpl actual = MappingsImpl.fromRawString("value"); - assertThat(actual).containsExactly(MappingImpl.address("value")); + assertThat(actual).containsOnly(MappingImpl.address("value")); } @Test public void fromRawStringShouldReturnCollectionWhenSeveralElementsString() { MappingsImpl actual = MappingsImpl.fromRawString("value1;value2"); - assertThat(actual).containsExactly(MappingImpl.address("value1"), MappingImpl.address("value2")); + assertThat(actual).containsOnly(MappingImpl.address("value1"), MappingImpl.address("value2")); } @Test public void fromRawStringShouldReturnSingleElementCollectionWhenTrailingDelimiterString() { MappingsImpl actual = MappingsImpl.fromRawString("value1;"); - assertThat(actual).containsExactly(MappingImpl.address("value1")); + assertThat(actual).containsOnly(MappingImpl.address("value1")); } @Test public void fromRawStringShouldReturnSingleElementCollectionWhenHeadingDelimiterString() { MappingsImpl actual = MappingsImpl.fromRawString(";value1"); - assertThat(actual).containsExactly(MappingImpl.address("value1")); + assertThat(actual).containsOnly(MappingImpl.address("value1")); } @Test public void fromRawStringShouldTrimValues() { MappingsImpl actual = MappingsImpl.fromRawString("value1 ; value2 "); - assertThat(actual).containsExactly(MappingImpl.address("value1"), MappingImpl.address("value2")); + assertThat(actual).containsOnly(MappingImpl.address("value1"), MappingImpl.address("value2")); } @Test public void fromRawStringShouldNotSkipEmptyValue() { MappingsImpl actual = MappingsImpl.fromRawString("value1; ;value2"); - assertThat(actual).containsExactly(MappingImpl.address("value1"), MappingImpl.address(""), MappingImpl.address("value2")); + assertThat(actual).containsOnly(MappingImpl.address("value1"), MappingImpl.address(""), MappingImpl.address("value2")); } @Test public void fromRawStringShouldReturnCollectionWhenValueContainsCommaSeperatedValues() { MappingsImpl actual = MappingsImpl.fromRawString("value1,value2"); - assertThat(actual).containsExactly(MappingImpl.address("value1"),MappingImpl.address("value2")); + assertThat(actual).containsOnly(MappingImpl.address("value1"),MappingImpl.address("value2")); } @Test public void fromRawStringShouldReturnCollectionWhenValueContainsColonSeperatedValues() { MappingsImpl actual = MappingsImpl.fromRawString("value1:value2"); - assertThat(actual).containsExactly(MappingImpl.address("value1"),MappingImpl.address("value2")); + assertThat(actual).containsOnly(MappingImpl.address("value1"),MappingImpl.address("value2")); } @Test public void fromRawStringShouldUseCommaDelimiterBeforeSemicolonWhenValueContainsBoth() { MappingsImpl actual = MappingsImpl.fromRawString("value1;value1,value2"); - assertThat(actual).containsExactly(MappingImpl.address("value1;value1"),MappingImpl.address("value2")); + assertThat(actual).containsOnly(MappingImpl.address("value1;value1"),MappingImpl.address("value2")); } @Test public void fromRawStringShouldUseSemicolonDelimiterBeforeColonWhenValueContainsBoth() { MappingsImpl actual = MappingsImpl.fromRawString("value1:value1;value2"); - assertThat(actual).containsExactly(MappingImpl.address("value1:value1"),MappingImpl.address("value2")); + assertThat(actual).containsOnly(MappingImpl.address("value1:value1"),MappingImpl.address("value2")); } @Test public void fromRawStringShouldNotUseColonDelimiterWhenValueStartsWithError() { MappingsImpl actual = MappingsImpl.fromRawString("error:test"); - assertThat(actual).containsExactly(MappingImpl.error("test")); + assertThat(actual).containsOnly(MappingImpl.error("test")); } @Test public void fromRawStringShouldNotUseColonDelimiterWhenValueStartsWithDomain() { MappingsImpl actual = MappingsImpl.fromRawString("domain:test"); - assertThat(actual).containsExactly(MappingImpl.domain(Domain.of("test"))); + assertThat(actual).containsOnly(MappingImpl.domain(Domain.of("test"))); } @Test public void fromRawStringShouldNotUseColonDelimiterWhenValueStartsWithRegex() { MappingsImpl actual = MappingsImpl.fromRawString("regex:test"); - assertThat(actual).containsExactly(MappingImpl.regex("test")); + assertThat(actual).containsOnly(MappingImpl.regex("test")); } @Test @@ -252,7 +252,7 @@ public class MappingsImplTest { @Test public void unionShouldReturnMergedWhenBothContainsData() { Mappings mappings = MappingsImpl.fromRawString("toto").union(MappingsImpl.fromRawString("tata")); - assertThat(mappings).containsExactly(MappingImpl.address("toto"),MappingImpl.address("tata")); + assertThat(mappings).containsOnly(MappingImpl.address("toto"),MappingImpl.address("tata")); } @Test @@ -286,7 +286,7 @@ public class MappingsImplTest { MappingsImpl mappingsImpl = MappingsImpl.Builder .merge(left, empty) .build(); - assertThat(mappingsImpl).containsExactly(expectedMapping); + assertThat(mappingsImpl).containsOnly(expectedMapping); } @Test @@ -297,7 +297,7 @@ public class MappingsImplTest { MappingsImpl mappingsImpl = MappingsImpl.Builder .merge(empty, right) .build(); - assertThat(mappingsImpl).containsExactly(expectedMapping); + assertThat(mappingsImpl).containsOnly(expectedMapping); } @Test @@ -309,6 +309,21 @@ public class MappingsImplTest { MappingsImpl mappingsImpl = MappingsImpl.Builder .merge(left, right) .build(); - assertThat(mappingsImpl).containsExactly(leftMapping, rightMapping); + assertThat(mappingsImpl).containsOnly(leftMapping, rightMapping); + } + + @Test + public void builderShouldPutDomainAliasFirstWhenVariousMappings() { + MappingImpl addressMapping = MappingImpl.address("aaa"); + MappingImpl errorMapping = MappingImpl.error("error"); + MappingImpl domainMapping = MappingImpl.domain(Domain.of("domain")); + MappingImpl domain2Mapping = MappingImpl.domain(Domain.of("domain2")); + MappingsImpl mappingsImpl = MappingsImpl.builder() + .add(domainMapping) + .add(addressMapping) + .add(errorMapping) + .add(domain2Mapping) + .build(); + assertThat(mappingsImpl).containsExactly(domainMapping, domain2Mapping, addressMapping, errorMapping); } } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org