JAMES-2366 Push strong typing in RRT implementation Get ride of **mapAddressInternal**. This also remove some useless mapping -> String -> mapping conversion
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/25589cb8 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/25589cb8 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/25589cb8 Branch: refs/heads/master Commit: 25589cb882157a8398548b16641397fd2492e4d1 Parents: d2a151b Author: benwa <btell...@linagora.com> Authored: Fri Apr 6 09:42:59 2018 +0700 Committer: benwa <btell...@linagora.com> Committed: Thu Apr 19 11:12:06 2018 +0700 ---------------------------------------------------------------------- .../CassandraRecipientRewriteTable.java | 6 +-- .../rrt/file/XMLRecipientRewriteTable.java | 12 +++--- .../rrt/hbase/HBaseRecipientRewriteTable.java | 4 +- .../rrt/jdbc/JDBCRecipientRewriteTable.java | 6 +-- .../james/rrt/jpa/JPARecipientRewriteTable.java | 10 ++--- .../rrt/lib/AbstractRecipientRewriteTable.java | 39 ++------------------ .../lib/AbstractRecipientRewriteTableTest.java | 30 ++++++++++----- .../rrt/memory/MemoryRecipientRewriteTable.java | 6 +-- 8 files changed, 44 insertions(+), 69 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/server/data/data-cassandra/src/main/java/org/apache/james/rrt/cassandra/CassandraRecipientRewriteTable.java ---------------------------------------------------------------------- diff --git a/server/data/data-cassandra/src/main/java/org/apache/james/rrt/cassandra/CassandraRecipientRewriteTable.java b/server/data/data-cassandra/src/main/java/org/apache/james/rrt/cassandra/CassandraRecipientRewriteTable.java index 0a2b098..c41ae1b 100644 --- a/server/data/data-cassandra/src/main/java/org/apache/james/rrt/cassandra/CassandraRecipientRewriteTable.java +++ b/server/data/data-cassandra/src/main/java/org/apache/james/rrt/cassandra/CassandraRecipientRewriteTable.java @@ -176,13 +176,11 @@ public class CassandraRecipientRewriteTable extends AbstractRecipientRewriteTabl } @Override - protected String mapAddressInternal(String user, Domain domain) { - Mappings mappings = OptionalUtils.orSuppliers( + protected Mappings mapAddress(String user, Domain domain) { + return OptionalUtils.orSuppliers( () -> retrieveMappings(user, domain), () -> retrieveMappings(WILDCARD, domain)) .orElse(MappingsImpl.empty()); - - return !mappings.isEmpty() ? mappings.serialize() : null; } } http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/server/data/data-file/src/main/java/org/apache/james/rrt/file/XMLRecipientRewriteTable.java ---------------------------------------------------------------------- diff --git a/server/data/data-file/src/main/java/org/apache/james/rrt/file/XMLRecipientRewriteTable.java b/server/data/data-file/src/main/java/org/apache/james/rrt/file/XMLRecipientRewriteTable.java index f543ece..de67ca7 100644 --- a/server/data/data-file/src/main/java/org/apache/james/rrt/file/XMLRecipientRewriteTable.java +++ b/server/data/data-file/src/main/java/org/apache/james/rrt/file/XMLRecipientRewriteTable.java @@ -20,6 +20,7 @@ package org.apache.james.rrt.file; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.HierarchicalConfiguration; @@ -57,12 +58,11 @@ public class XMLRecipientRewriteTable extends AbstractRecipientRewriteTable { } @Override - protected String mapAddressInternal(String user, Domain domain) throws RecipientRewriteTableException { - if (mappings == null) { - return null; - } else { - return RecipientRewriteTableUtil.getTargetString(user, domain, mappings); - } + protected Mappings mapAddress(String user, Domain domain) throws RecipientRewriteTableException { + return Optional.ofNullable(mappings) + .map(mappings -> RecipientRewriteTableUtil.getTargetString(user, domain, mappings)) + .map(MappingsImpl::fromRawString) + .orElse(MappingsImpl.empty()); } @Override http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/server/data/data-hbase/src/main/java/org/apache/james/rrt/hbase/HBaseRecipientRewriteTable.java ---------------------------------------------------------------------- diff --git a/server/data/data-hbase/src/main/java/org/apache/james/rrt/hbase/HBaseRecipientRewriteTable.java b/server/data/data-hbase/src/main/java/org/apache/james/rrt/hbase/HBaseRecipientRewriteTable.java index 7e019b5..98301b5 100644 --- a/server/data/data-hbase/src/main/java/org/apache/james/rrt/hbase/HBaseRecipientRewriteTable.java +++ b/server/data/data-hbase/src/main/java/org/apache/james/rrt/hbase/HBaseRecipientRewriteTable.java @@ -157,7 +157,7 @@ public class HBaseRecipientRewriteTable extends AbstractRecipientRewriteTable { } @Override - protected String mapAddressInternal(String user, Domain domain) throws RecipientRewriteTableException { + protected Mappings mapAddress(String user, Domain domain) throws RecipientRewriteTableException { HTableInterface table = null; String mappings = null; try { @@ -181,7 +181,7 @@ public class HBaseRecipientRewriteTable extends AbstractRecipientRewriteTable { } } } - return mappings; + return MappingsImpl.fromRawString(mappings); } private String getMapping(HTableInterface table, String user, Domain domain) throws IOException { http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/server/data/data-jdbc/src/main/java/org/apache/james/rrt/jdbc/JDBCRecipientRewriteTable.java ---------------------------------------------------------------------- diff --git a/server/data/data-jdbc/src/main/java/org/apache/james/rrt/jdbc/JDBCRecipientRewriteTable.java b/server/data/data-jdbc/src/main/java/org/apache/james/rrt/jdbc/JDBCRecipientRewriteTable.java index 9c9ffba..eb9ae06 100644 --- a/server/data/data-jdbc/src/main/java/org/apache/james/rrt/jdbc/JDBCRecipientRewriteTable.java +++ b/server/data/data-jdbc/src/main/java/org/apache/james/rrt/jdbc/JDBCRecipientRewriteTable.java @@ -197,7 +197,7 @@ public class JDBCRecipientRewriteTable extends AbstractRecipientRewriteTable { } @Override - protected String mapAddressInternal(String user, Domain domain) throws RecipientRewriteTableException { + protected Mappings mapAddress(String user, Domain domain) throws RecipientRewriteTableException { Connection conn = null; PreparedStatement mappingStmt = null; try { @@ -210,7 +210,7 @@ public class JDBCRecipientRewriteTable extends AbstractRecipientRewriteTable { mappingStmt.setString(2, domain.asString()); mappingRS = mappingStmt.executeQuery(); if (mappingRS.next()) { - return mappingRS.getString(1); + return MappingsImpl.fromRawString(mappingRS.getString(1)); } } finally { theJDBCUtil.closeJDBCResultSet(mappingRS); @@ -223,7 +223,7 @@ public class JDBCRecipientRewriteTable extends AbstractRecipientRewriteTable { theJDBCUtil.closeJDBCStatement(mappingStmt); theJDBCUtil.closeJDBCConnection(conn); } - return null; + return MappingsImpl.empty(); } @Override http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/JPARecipientRewriteTable.java ---------------------------------------------------------------------- diff --git a/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/JPARecipientRewriteTable.java b/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/JPARecipientRewriteTable.java index 44c5c9f..87a6fa7 100644 --- a/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/JPARecipientRewriteTable.java +++ b/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/JPARecipientRewriteTable.java @@ -76,15 +76,15 @@ public class JPARecipientRewriteTable extends AbstractRecipientRewriteTable { } @Override - protected String mapAddressInternal(String user, Domain domain) throws RecipientRewriteTableException { - String mapping = getMapping(user, domain, "selectExactMappings"); + protected Mappings mapAddress(String user, Domain domain) throws RecipientRewriteTableException { + Mappings mapping = getMapping(user, domain, "selectExactMappings"); if (mapping != null) { return mapping; } return getMapping(user, domain, "selectMappings"); } - private String getMapping(String user, Domain domain, String queryName) throws RecipientRewriteTableException { + private Mappings getMapping(String user, Domain domain, String queryName) throws RecipientRewriteTableException { EntityManager entityManager = entityManagerFactory.createEntityManager(); final EntityTransaction transaction = entityManager.getTransaction(); try { @@ -97,7 +97,7 @@ public class JPARecipientRewriteTable extends AbstractRecipientRewriteTable { .getResultList(); transaction.commit(); if (virtualUsers.size() > 0) { - return virtualUsers.get(0).getTargetAddress(); + return MappingsImpl.fromRawString(virtualUsers.get(0).getTargetAddress()); } } catch (PersistenceException e) { LOGGER.debug("Failed to find mapping for user={} and domain={}", user, domain, e); @@ -108,7 +108,7 @@ public class JPARecipientRewriteTable extends AbstractRecipientRewriteTable { } finally { entityManager.close(); } - return null; + return MappingsImpl.empty(); } @Override http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/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 8b314b1..e1b9035 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 @@ -390,42 +390,11 @@ public abstract class AbstractRecipientRewriteTable implements RecipientRewriteT protected abstract Map<String, Mappings> getAllMappingsInternal() throws RecipientRewriteTableException; /** - * Override to map virtual recipients to real recipients, both local and - * non-local. Each key in the provided map corresponds to a potential - * virtual recipient, stored as a <code>MailAddress</code> object. - * - * Translate virtual recipients to real recipients by mapping a string - * containing the address of the real recipient as a value to a key. Leave - * the value <code>null<code> - * if no mapping should be performed. Multiple recipients may be specified by delineating - * the mapped string with commas, semi-colons or colons. - * - * @param user - * the mapping of virtual to real recipients, as - * <code>MailAddress</code>es to <code>String</code>s. - */ - protected abstract String mapAddressInternal(String user, Domain domain) throws RecipientRewriteTableException; - - /** - * Get all mappings for the given user and domain. If a aliasdomain mapping - * was found get sure it is in the map as first mapping. - * - * @param user - * the username - * @param domain - * the domain - * @return the mappings + * This method must return stored Mappings for the given user. + * It must never return null but throw RecipientRewriteTableException on errors and return an empty Mappings + * object if no mapping is found. */ - private Mappings mapAddress(String user, Domain domain) throws RecipientRewriteTableException { - - String mappings = mapAddressInternal(user, domain); - - if (mappings != null) { - return MappingsImpl.fromRawString(mappings); - } else { - return null; - } - } + protected abstract Mappings mapAddress(String user, Domain domain) throws RecipientRewriteTableException; private void checkMapping(String user, Domain domain, Mapping mapping) throws RecipientRewriteTableException { Mappings mappings = getUserDomainMappings(user, domain); http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/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 69b6d28..f779524 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 @@ -93,7 +93,8 @@ public abstract class AbstractRecipientRewriteTableTest { String regex2 = "(.+)@test"; String invalidRegex = ".*):"; - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); addMapping(user, domain, regex, Type.Regex); addMapping(user, domain, regex2, Type.Regex); @@ -109,7 +110,8 @@ public abstract class AbstractRecipientRewriteTableTest { removeMapping(user, domain, regex2, Type.Regex); - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); assertThat(virtualUserTable.getAllMappings()).describedAs("No mapping").isNull(); } @@ -145,7 +147,8 @@ public abstract class AbstractRecipientRewriteTableTest { String address = "test@localhost2"; String address2 = "test@james"; - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); addMapping(user, domain, address, Type.Address); addMapping(user, domain, address2, Type.Address); @@ -157,7 +160,8 @@ public abstract class AbstractRecipientRewriteTableTest { removeMapping(user, domain, address2, Type.Address); - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); assertThat(virtualUserTable.getAllMappings()).describedAs("No mapping").isNull(); } @@ -167,7 +171,8 @@ public abstract class AbstractRecipientRewriteTableTest { Domain domain = Domain.LOCALHOST; String error = "bounce!"; - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); addMapping(user, domain, error, Type.Error); assertThat(virtualUserTable.getAllMappings()).describedAs("One mappingline").hasSize(1); @@ -179,7 +184,8 @@ public abstract class AbstractRecipientRewriteTableTest { removeMapping(user, domain, error, Type.Error); - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); assertThat(virtualUserTable.getAllMappings()).describedAs("No mapping").isNull(); } @@ -191,7 +197,8 @@ public abstract class AbstractRecipientRewriteTableTest { String address = "test@localhost2"; String address2 = "test@james"; - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); addMapping(RecipientRewriteTable.WILDCARD, domain, address, Type.Address); addMapping(user, domain, address2, Type.Address); @@ -202,8 +209,10 @@ public abstract class AbstractRecipientRewriteTableTest { removeMapping(user, domain, address2, Type.Address); removeMapping(RecipientRewriteTable.WILDCARD, domain, address, Type.Address); - assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping").isNull(); - assertThat(virtualUserTable.getMappings(user2, domain)).describedAs("No mapping").isNull(); + assertThat(virtualUserTable.getMappings(user, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); + assertThat(virtualUserTable.getMappings(user2, domain)).describedAs("No mapping") + .isEqualTo(MappingsImpl.empty()); } @Test @@ -309,7 +318,8 @@ public abstract class AbstractRecipientRewriteTableTest { removeMapping(user, domain, address, Type.Forward); removeMapping(user, domain, address2, Type.Forward); - assertThat(virtualUserTable.getMappings(user, domain)).isNull(); + assertThat(virtualUserTable.getMappings(user, domain)) + .isEqualTo(MappingsImpl.empty()); } protected abstract AbstractRecipientRewriteTable getRecipientRewriteTable() throws Exception; http://git-wip-us.apache.org/repos/asf/james-project/blob/25589cb8/server/data/data-memory/src/main/java/org/apache/james/rrt/memory/MemoryRecipientRewriteTable.java ---------------------------------------------------------------------- diff --git a/server/data/data-memory/src/main/java/org/apache/james/rrt/memory/MemoryRecipientRewriteTable.java b/server/data/data-memory/src/main/java/org/apache/james/rrt/memory/MemoryRecipientRewriteTable.java index c8d3965..761373b 100644 --- a/server/data/data-memory/src/main/java/org/apache/james/rrt/memory/MemoryRecipientRewriteTable.java +++ b/server/data/data-memory/src/main/java/org/apache/james/rrt/memory/MemoryRecipientRewriteTable.java @@ -109,13 +109,11 @@ public class MemoryRecipientRewriteTable extends AbstractRecipientRewriteTable { } @Override - protected String mapAddressInternal(String user, Domain domain) { - Mappings mappings = OptionalUtils.orSuppliers( + protected Mappings mapAddress(String user, Domain domain) { + return OptionalUtils.orSuppliers( () -> retrieveMappings(user, domain), () -> retrieveMappings(WILDCARD, domain)) .orElse(MappingsImpl.empty()); - - return !mappings.isEmpty() ? mappings.serialize() : null; } @Override --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org