From 5408d21adabe37555f1d1a5c0f8e87ebc90ea00d Mon Sep 17 00:00:00 2001 From: amichair Date: Mon, 14 Oct 2024 07:45:16 +0300 Subject: [PATCH] James 1409 - Change JPARecipientRewriteTable to store separate record per target address (#2444) --- .../modules/server/DataRoutesModules.java | 4 +- .../file/XMLRecipientRewriteTableTest.java | 6 + .../rrt/jpa/JPARecipientRewriteTable.java | 172 ++++++------------ .../rrt/jpa/model/JPARecipientRewrite.java | 21 ++- .../lib/RecipientRewriteTableContract.java | 13 ++ .../james/webadmin/utils/JsonExtractor.java | 27 ++- .../webadmin/dto/MappingSourceModule.java | 51 ------ .../james/webadmin/dto/MappingValueDTO.java | 46 ----- .../james/webadmin/dto/MappingsModule.java | 132 ++++++++++++++ .../james/webadmin/routes/MappingRoutes.java | 63 ++++--- .../webadmin/routes/AliasRoutesTest.java | 5 +- .../webadmin/routes/ForwardRoutesTest.java | 5 +- .../webadmin/routes/GroupsRoutesTest.java | 5 +- .../webadmin/routes/MappingRoutesTest.java | 51 +++++- upgrade-instructions.md | 21 +++ 15 files changed, 363 insertions(+), 259 deletions(-) delete mode 100644 server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingSourceModule.java delete mode 100644 server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingValueDTO.java create mode 100644 server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingsModule.java diff --git a/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java b/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java index 48fcf5efe27..d161e326b7a 100644 --- a/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java +++ b/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java @@ -28,7 +28,7 @@ import org.apache.james.user.api.UsersRepository; import org.apache.james.webadmin.Routes; import org.apache.james.webadmin.dto.DTOModuleInjections; -import org.apache.james.webadmin.dto.MappingSourceModule; +import org.apache.james.webadmin.dto.MappingsModule; import org.apache.james.webadmin.mdc.RequestLogger; import org.apache.james.webadmin.routes.AddressMappingRoutes; import org.apache.james.webadmin.routes.AliasRoutes; @@ -75,7 +75,7 @@ protected void configure() { routesMultibinder.addBinding().to(DeleteUserDataRoutes.class); Multibinder jsonTransformerModuleMultibinder = Multibinder.newSetBinder(binder(), JsonTransformerModule.class); - jsonTransformerModuleMultibinder.addBinding().to(MappingSourceModule.class); + jsonTransformerModuleMultibinder.addBinding().to(MappingsModule.class); Multibinder.newSetBinder(binder(), RequestLogger.class).addBinding().to(UserCreationRequestLogger.class); } diff --git a/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java b/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java index 4a206f94401..01c6aee368f 100644 --- a/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java +++ b/server/data/data-file/src/test/java/org/apache/james/rrt/file/XMLRecipientRewriteTableTest.java @@ -162,6 +162,12 @@ public void listSourcesShouldReturnWhenMultipleSourceMapping() { } + @Test + @Disabled("XMLRecipientRewriteTable is read only") + public void listSourcesShouldReturnWhenSourceHasMultipleMappings() { + + } + @Test @Disabled("XMLRecipientRewriteTable is read only") public void listSourcesShouldReturnWhenHasForwardMapping() { 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 0a6990cde67..9ba6b3500db 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 @@ -22,11 +22,10 @@ import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.SELECT_ALL_MAPPINGS_QUERY; import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.SELECT_SOURCES_BY_MAPPING_QUERY; import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.SELECT_USER_DOMAIN_MAPPING_QUERY; -import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.UPDATE_MAPPING_QUERY; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import java.util.stream.Stream; import jakarta.inject.Inject; @@ -73,12 +72,44 @@ public void setEntityManagerFactory(EntityManagerFactory entityManagerFactory) { @Override public void addMapping(MappingSource source, Mapping mapping) throws RecipientRewriteTableException { - Mappings map = getStoredMappings(source); - if (!map.isEmpty()) { - Mappings updatedMappings = MappingsImpl.from(map).add(mapping).build(); - doUpdateMapping(source, updatedMappings.serialize()); - } else { - doAddMapping(source, mapping.asString()); + EntityManager entityManager = entityManagerFactory.createEntityManager(); + final EntityTransaction transaction = entityManager.getTransaction(); + try { + JPARecipientRewrite jpaRecipientRewrite = new JPARecipientRewrite(source.getFixedUser(), Domain.of(source.getFixedDomain()), mapping.asString()); + transaction.begin(); + entityManager.merge(jpaRecipientRewrite); + transaction.commit(); + } catch (PersistenceException e) { + LOGGER.debug("Failed to save virtual user", e); + if (transaction.isActive()) { + transaction.rollback(); + } + throw new RecipientRewriteTableException("Unable to add mapping", e); + } finally { + EntityManagerUtils.safelyClose(entityManager); + } + } + + @Override + public void removeMapping(MappingSource source, Mapping mapping) throws RecipientRewriteTableException { + EntityManager entityManager = entityManagerFactory.createEntityManager(); + final EntityTransaction transaction = entityManager.getTransaction(); + try { + transaction.begin(); + entityManager.createNamedQuery(DELETE_MAPPING_QUERY) + .setParameter("user", source.getFixedUser()) + .setParameter("domain", source.getFixedDomain()) + .setParameter("targetAddress", mapping.asString()) + .executeUpdate(); + transaction.commit(); + } catch (PersistenceException e) { + LOGGER.debug("Failed to remove mapping", e); + if (transaction.isActive()) { + transaction.rollback(); + } + throw new RecipientRewriteTableException("Unable to remove mapping", e); + } finally { + EntityManagerUtils.safelyClose(entityManager); } } @@ -96,18 +127,17 @@ protected Mappings mapAddress(String user, Domain domain) throws RecipientRewrit } @Override + @SuppressWarnings("unchecked") public Mappings getStoredMappings(MappingSource source) throws RecipientRewriteTableException { EntityManager entityManager = entityManagerFactory.createEntityManager(); try { - @SuppressWarnings("unchecked") - List virtualUsers = entityManager.createNamedQuery(SELECT_USER_DOMAIN_MAPPING_QUERY) + List mappings = (List) entityManager.createNamedQuery(SELECT_USER_DOMAIN_MAPPING_QUERY) .setParameter("user", source.getFixedUser()) .setParameter("domain", source.getFixedDomain()) - .getResultList(); - if (virtualUsers.size() > 0) { - return MappingsImpl.fromRawString(virtualUsers.get(0).getTargetAddress()); - } - return MappingsImpl.empty(); + .getResultStream() + .map(r -> Mapping.of((((JPARecipientRewrite)r).getTargetAddress()))) + .collect(Collectors.toList()); + return MappingsImpl.fromMappings(mappings.stream()); } catch (PersistenceException e) { LOGGER.debug("Failed to get user domain mappings", e); throw new RecipientRewriteTableException("Error while retrieve mappings", e); @@ -117,16 +147,16 @@ public Mappings getStoredMappings(MappingSource source) throws RecipientRewriteT } @Override + @SuppressWarnings("unchecked") public Map getAllMappings() throws RecipientRewriteTableException { EntityManager entityManager = entityManagerFactory.createEntityManager(); - Map mapping = new HashMap<>(); try { - @SuppressWarnings("unchecked") - List virtualUsers = entityManager.createNamedQuery(SELECT_ALL_MAPPINGS_QUERY).getResultList(); - for (JPARecipientRewrite virtualUser : virtualUsers) { - mapping.put(MappingSource.fromUser(virtualUser.getUser(), virtualUser.getDomain()), MappingsImpl.fromRawString(virtualUser.getTargetAddress())); - } - return mapping; + return (Map) entityManager.createNamedQuery(SELECT_ALL_MAPPINGS_QUERY) + .getResultStream() + .collect(Collectors.toMap( + r -> MappingSource.fromUser(((JPARecipientRewrite)r).getUser(), ((JPARecipientRewrite)r).getDomain()), + r -> MappingsImpl.fromRawString(((JPARecipientRewrite)r).getTargetAddress()), + (m1, m2) -> MappingsImpl.from(m1).addAll(m2).build())); } catch (PersistenceException e) { LOGGER.debug("Failed to get all mappings", e); throw new RecipientRewriteTableException("Error while retrieve mappings", e); @@ -144,9 +174,8 @@ public Stream listSources(Mapping mapping) throws RecipientRewrit try { return entityManager.createNamedQuery(SELECT_SOURCES_BY_MAPPING_QUERY, JPARecipientRewrite.class) .setParameter("targetAddress", mapping.asString()) - .getResultList() - .stream() - .map(user -> MappingSource.fromUser(user.getUser(), user.getDomain())); + .getResultStream() + .map(r -> MappingSource.fromUser(r.getUser(), r.getDomain())); } catch (PersistenceException e) { String error = "Unable to list sources by mapping"; LOGGER.debug(error, e); @@ -155,97 +184,4 @@ public Stream listSources(Mapping mapping) throws RecipientRewrit EntityManagerUtils.safelyClose(entityManager); } } - - @Override - public void removeMapping(MappingSource source, Mapping mapping) throws RecipientRewriteTableException { - Mappings map = getStoredMappings(source); - if (map.size() > 1) { - Mappings updatedMappings = map.remove(mapping); - doUpdateMapping(source, updatedMappings.serialize()); - } else { - doRemoveMapping(source, mapping.asString()); - } - } - - /** - * Update the mapping for the given user and domain - * - * @return true if update was successfully - */ - private boolean doUpdateMapping(MappingSource source, String mapping) throws RecipientRewriteTableException { - EntityManager entityManager = entityManagerFactory.createEntityManager(); - final EntityTransaction transaction = entityManager.getTransaction(); - try { - transaction.begin(); - int updated = entityManager - .createNamedQuery(UPDATE_MAPPING_QUERY) - .setParameter("targetAddress", mapping) - .setParameter("user", source.getFixedUser()) - .setParameter("domain", source.getFixedDomain()) - .executeUpdate(); - transaction.commit(); - if (updated > 0) { - return true; - } - } catch (PersistenceException e) { - LOGGER.debug("Failed to update mapping", e); - if (transaction.isActive()) { - transaction.rollback(); - } - throw new RecipientRewriteTableException("Unable to update mapping", e); - } finally { - EntityManagerUtils.safelyClose(entityManager); - } - return false; - } - - /** - * Remove a mapping for the given user and domain - */ - private void doRemoveMapping(MappingSource source, String mapping) throws RecipientRewriteTableException { - EntityManager entityManager = entityManagerFactory.createEntityManager(); - final EntityTransaction transaction = entityManager.getTransaction(); - try { - transaction.begin(); - entityManager.createNamedQuery(DELETE_MAPPING_QUERY) - .setParameter("user", source.getFixedUser()) - .setParameter("domain", source.getFixedDomain()) - .setParameter("targetAddress", mapping) - .executeUpdate(); - transaction.commit(); - - } catch (PersistenceException e) { - LOGGER.debug("Failed to remove mapping", e); - if (transaction.isActive()) { - transaction.rollback(); - } - throw new RecipientRewriteTableException("Unable to remove mapping", e); - - } finally { - EntityManagerUtils.safelyClose(entityManager); - } - } - - /** - * Add mapping for given user and domain - */ - private void doAddMapping(MappingSource source, String mapping) throws RecipientRewriteTableException { - EntityManager entityManager = entityManagerFactory.createEntityManager(); - final EntityTransaction transaction = entityManager.getTransaction(); - try { - transaction.begin(); - JPARecipientRewrite jpaRecipientRewrite = new JPARecipientRewrite(source.getFixedUser(), Domain.of(source.getFixedDomain()), mapping); - entityManager.persist(jpaRecipientRewrite); - transaction.commit(); - } catch (PersistenceException e) { - LOGGER.debug("Failed to save virtual user", e); - if (transaction.isActive()) { - transaction.rollback(); - } - throw new RecipientRewriteTableException("Unable to add mapping", e); - } finally { - EntityManagerUtils.safelyClose(entityManager); - } - } - } diff --git a/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/model/JPARecipientRewrite.java b/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/model/JPARecipientRewrite.java index ee0f5185fa7..e0ee8f63a20 100644 --- a/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/model/JPARecipientRewrite.java +++ b/server/data/data-jpa/src/main/java/org/apache/james/rrt/jpa/model/JPARecipientRewrite.java @@ -22,7 +22,6 @@ import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.SELECT_ALL_MAPPINGS_QUERY; import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.SELECT_SOURCES_BY_MAPPING_QUERY; import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.SELECT_USER_DOMAIN_MAPPING_QUERY; -import static org.apache.james.rrt.jpa.model.JPARecipientRewrite.UPDATE_MAPPING_QUERY; import java.io.Serializable; @@ -30,6 +29,7 @@ import jakarta.persistence.Entity; import jakarta.persistence.Id; import jakarta.persistence.IdClass; +import jakarta.persistence.Index; import jakarta.persistence.NamedQuery; import jakarta.persistence.Table; @@ -42,36 +42,40 @@ * persistence. */ @Entity(name = "JamesRecipientRewrite") -@Table(name = JPARecipientRewrite.JAMES_RECIPIENT_REWRITE) +@Table(name = JPARecipientRewrite.JAMES_RECIPIENT_REWRITE, indexes = { + // note: the generated primary key index also includes these 3 fields, but in unuseful alphabetic order + @Index(name = "USER_NAME_DOMAIN_NAME_TARGET_ADDRESS_INDEX", columnList = "USER_NAME,DOMAIN_NAME,TARGET_ADDRESS"), + @Index(name = "TARGET_ADDRESS_INDEX", columnList = "TARGET_ADDRESS") +}) @NamedQuery(name = SELECT_USER_DOMAIN_MAPPING_QUERY, query = "SELECT rrt FROM JamesRecipientRewrite rrt WHERE rrt.user=:user AND rrt.domain=:domain") @NamedQuery(name = SELECT_ALL_MAPPINGS_QUERY, query = "SELECT rrt FROM JamesRecipientRewrite rrt") @NamedQuery(name = DELETE_MAPPING_QUERY, query = "DELETE FROM JamesRecipientRewrite rrt WHERE rrt.user=:user AND rrt.domain=:domain AND rrt.targetAddress=:targetAddress") -@NamedQuery(name = UPDATE_MAPPING_QUERY, query = "UPDATE JamesRecipientRewrite rrt SET rrt.targetAddress=:targetAddress WHERE rrt.user=:user AND rrt.domain=:domain") @NamedQuery(name = SELECT_SOURCES_BY_MAPPING_QUERY, query = "SELECT rrt FROM JamesRecipientRewrite rrt WHERE rrt.targetAddress=:targetAddress") @IdClass(JPARecipientRewrite.RecipientRewriteTableId.class) public class JPARecipientRewrite { public static final String SELECT_USER_DOMAIN_MAPPING_QUERY = "selectUserDomainMapping"; public static final String SELECT_ALL_MAPPINGS_QUERY = "selectAllMappings"; public static final String DELETE_MAPPING_QUERY = "deleteMapping"; - public static final String UPDATE_MAPPING_QUERY = "updateMapping"; public static final String SELECT_SOURCES_BY_MAPPING_QUERY = "selectSourcesByMapping"; public static final String JAMES_RECIPIENT_REWRITE = "JAMES_RECIPIENT_REWRITE"; public static class RecipientRewriteTableId implements Serializable { - private static final long serialVersionUID = 1L; + private static final long serialVersionUID = 2L; private String user; private String domain; + private String targetAddress; + public RecipientRewriteTableId() { } @Override public int hashCode() { - return Objects.hashCode(user, domain); + return Objects.hashCode(user, domain, targetAddress); } @Override @@ -83,7 +87,9 @@ public boolean equals(Object obj) { return false; } final RecipientRewriteTableId other = (RecipientRewriteTableId) obj; - return Objects.equal(this.user, other.user) && Objects.equal(this.domain, other.domain); + return Objects.equal(this.user, other.user) + && Objects.equal(this.domain, other.domain) + && Objects.equal(this.targetAddress, other.targetAddress); } } @@ -106,6 +112,7 @@ public boolean equals(Object obj) { * The target address. column name is chosen to be compatible with the * JDBCRecipientRewriteTableList. */ + @Id @Column(name = "TARGET_ADDRESS", nullable = false, length = 100) private String targetAddress = ""; diff --git a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java index a2767895bf6..1acc5cab38a 100644 --- a/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java +++ b/server/data/data-library/src/test/java/org/apache/james/rrt/lib/RecipientRewriteTableContract.java @@ -411,6 +411,19 @@ default void listSourcesShouldReturnWhenMultipleSourceMapping() throws Exception assertThat(virtualUserTable().listSources(mapping)).contains(source, source2); } + @Test + default void listSourcesShouldReturnWhenSourceHasMultipleMappings() throws Exception { + MappingSource source = MappingSource.fromUser(USER, Domain.of("james")); + Mapping mapping = Mapping.group(ADDRESS); + Mapping mapping2 = Mapping.group(ADDRESS_2); + + virtualUserTable().addMapping(source, mapping); + virtualUserTable().addMapping(source, mapping2); + + assertThat(virtualUserTable().listSources(mapping)).contains(source); + assertThat(virtualUserTable().listSources(mapping2)).contains(source); + } + @Test default void listSourcesShouldReturnWhenHasForwardMapping() throws Exception { Mapping mapping = Mapping.forward("forward"); diff --git a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/JsonExtractor.java b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/JsonExtractor.java index edb71adb681..e81e0f2f950 100644 --- a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/JsonExtractor.java +++ b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/JsonExtractor.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.List; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.guava.GuavaModule; @@ -33,23 +34,37 @@ public class JsonExtractor { private final ObjectMapper objectMapper; private final Class type; + private final TypeReference typeReference; - public JsonExtractor(Class type, Module... modules) { - this(type, ImmutableList.copyOf(modules)); - } - - public JsonExtractor(Class type, List modules) { + private JsonExtractor(Class type, TypeReference typeReference, List modules) { this.objectMapper = new ObjectMapper() .registerModule(new Jdk8Module()) .registerModule(new GuavaModule()) .registerModules(modules); this.type = type; + this.typeReference = typeReference; + } + + public JsonExtractor(Class type, Module... modules) { + this(type, ImmutableList.copyOf(modules)); + } + + public JsonExtractor(Class type, List modules) { + this(type, null, modules); + } + + public JsonExtractor(TypeReference typeReference, Module... modules) { + this(typeReference, ImmutableList.copyOf(modules)); + } + + public JsonExtractor(TypeReference typeReference, List modules) { + this(null, typeReference, modules); } public RequestT parse(String text) throws JsonExtractException { Preconditions.checkNotNull(text); try { - return objectMapper.readValue(text, type); + return type != null ? objectMapper.readValue(text, type) : objectMapper.readValue(text, typeReference); } catch (IOException | IllegalArgumentException e) { throw new JsonExtractException(e); } diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingSourceModule.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingSourceModule.java deleted file mode 100644 index a8b41814dea..00000000000 --- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingSourceModule.java +++ /dev/null @@ -1,51 +0,0 @@ -/**************************************************************** - * 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.webadmin.dto; - -import java.io.IOException; - -import org.apache.james.rrt.lib.MappingSource; -import org.apache.james.webadmin.utils.JsonTransformerModule; - -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonSerializer; -import com.fasterxml.jackson.databind.Module; -import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.module.SimpleModule; - -public class MappingSourceModule implements JsonTransformerModule { - - private final SimpleModule simpleModule; - - public MappingSourceModule() { - simpleModule = new SimpleModule() - .addSerializer(MappingSource.class, new JsonSerializer() { - @Override - public void serialize(MappingSource mappingSource, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException { - jsonGenerator.writeString(mappingSource.asMailAddressString()); - } - }); - } - - @Override - public Module asJacksonModule() { - return simpleModule; - } -} \ No newline at end of file diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingValueDTO.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingValueDTO.java deleted file mode 100644 index ea75c261034..00000000000 --- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingValueDTO.java +++ /dev/null @@ -1,46 +0,0 @@ -/**************************************************************** - * 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.webadmin.dto; - -import org.apache.james.rrt.lib.Mapping; - -public class MappingValueDTO { - - public static MappingValueDTO fromMapping(Mapping mapping) { - return new MappingValueDTO(mapping.getType().toString(), mapping.getMappingValue()); - } - - private final String type; - private final String mapping; - - private MappingValueDTO(String type, String mapping) { - this.type = type; - this.mapping = mapping; - } - - public String getType() { - return type; - } - - public String getMapping() { - return mapping; - } - -} \ No newline at end of file diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingsModule.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingsModule.java new file mode 100644 index 00000000000..1f268543ca4 --- /dev/null +++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/dto/MappingsModule.java @@ -0,0 +1,132 @@ +/**************************************************************** + * 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.webadmin.dto; + +import java.io.IOException; +import java.util.List; + +import org.apache.james.rrt.lib.Mapping; +import org.apache.james.rrt.lib.MappingSource; +import org.apache.james.rrt.lib.Mappings; +import org.apache.james.rrt.lib.MappingsImpl; +import org.apache.james.webadmin.utils.JsonTransformerModule; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.JsonSerializer; +import com.fasterxml.jackson.databind.KeyDeserializer; +import com.fasterxml.jackson.databind.Module; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.module.SimpleModule; + +public class MappingsModule implements JsonTransformerModule { + + private static class MappingSourceSerializer extends JsonSerializer { + @Override + public void serialize(MappingSource mappingSource, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException { + jsonGenerator.writeString(mappingSource.asString()); + } + } + + private static class MappingSourceDeserializer extends JsonDeserializer { + @Override + public MappingSource deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { + return MappingSource.parse(jsonParser.getText()); + } + } + + private static class MappingSourceKeySerializer extends JsonSerializer { + @Override + public void serialize(MappingSource mappingSource, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException { + jsonGenerator.writeFieldName(mappingSource.asString()); + } + } + + private static class MappingSourceKeyDeserializer extends KeyDeserializer { + @Override + public Object deserializeKey(String key, DeserializationContext deserializationContext) throws IOException { + return MappingSource.parse(key); + } + } + + private static class MappingSerializer extends JsonSerializer { + @Override + public void serialize(Mapping mapping, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException { + jsonGenerator.writeStartObject(); + jsonGenerator.writeStringField("type", mapping.getType().toString()); + jsonGenerator.writeStringField("mapping", mapping.getMappingValue()); + jsonGenerator.writeEndObject(); + } + } + + private static class MappingDeserializer extends JsonDeserializer { + @Override + public Mapping deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { + JsonNode node = jsonParser.getCodec().readTree(jsonParser); + String type = node.get("type").asText(); + String mapping = node.get("mapping").asText(); + return Mapping.of(Mapping.Type.valueOf(type), mapping); + } + } + + private static class MappingsSerializer extends JsonSerializer { + @Override + public void serialize(Mappings mappings, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException { + jsonGenerator.writeStartArray(); + for (Mapping mapping : mappings) { + jsonGenerator.writeObject(mapping); + } + jsonGenerator.writeEndArray(); + } + } + + private static class MappingsDeserializer extends JsonDeserializer { + @Override + public Mappings deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { + ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec(); + List items = mapper.readValue(jsonParser, new TypeReference<>() {}); + return MappingsImpl.fromMappings(items.stream()); + } + } + + private final SimpleModule simpleModule; + + public MappingsModule() { + simpleModule = new SimpleModule() + .addSerializer(MappingSource.class, new MappingSourceSerializer()) + .addDeserializer(MappingSource.class, new MappingSourceDeserializer()) + .addKeySerializer(MappingSource.class, new MappingSourceKeySerializer()) + .addKeyDeserializer(MappingSource.class, new MappingSourceKeyDeserializer()) + .addSerializer(Mapping.class, new MappingSerializer()) + .addDeserializer(Mapping.class, new MappingDeserializer()) + .addSerializer(Mappings.class, new MappingsSerializer()) + .addDeserializer(Mappings.class, new MappingsDeserializer()); + } + + @Override + public Module asJacksonModule() { + return simpleModule; + } +} \ No newline at end of file diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/MappingRoutes.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/MappingRoutes.java index d6f85b4ac3c..8e33fe90188 100644 --- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/MappingRoutes.java +++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/MappingRoutes.java @@ -19,23 +19,26 @@ package org.apache.james.webadmin.routes; -import java.util.List; +import java.util.Map; import jakarta.inject.Inject; -import org.apache.commons.lang3.tuple.Pair; import org.apache.james.core.Username; import org.apache.james.rrt.api.RecipientRewriteTable; import org.apache.james.rrt.api.RecipientRewriteTableException; +import org.apache.james.rrt.lib.Mapping; import org.apache.james.rrt.lib.MappingSource; +import org.apache.james.rrt.lib.Mappings; import org.apache.james.webadmin.Routes; -import org.apache.james.webadmin.dto.MappingValueDTO; +import org.apache.james.webadmin.dto.MappingsModule; import org.apache.james.webadmin.utils.ErrorResponder; +import org.apache.james.webadmin.utils.JsonExtractException; +import org.apache.james.webadmin.utils.JsonExtractor; import org.apache.james.webadmin.utils.JsonTransformer; +import org.apache.james.webadmin.utils.Responses; import org.eclipse.jetty.http.HttpStatus; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; +import com.fasterxml.jackson.core.type.TypeReference; import spark.Request; import spark.Response; @@ -48,11 +51,14 @@ public class MappingRoutes implements Routes { static final String USER = "user"; private final JsonTransformer jsonTransformer; + private final JsonExtractor> jsonExtractor; private final RecipientRewriteTable recipientRewriteTable; @Inject MappingRoutes(JsonTransformer jsonTransformer, RecipientRewriteTable recipientRewriteTable) { this.jsonTransformer = jsonTransformer; + TypeReference> typeRef = new TypeReference<>() {}; + this.jsonExtractor = new JsonExtractor<>(typeRef, new MappingsModule().asJacksonModule()); this.recipientRewriteTable = recipientRewriteTable; } @@ -64,19 +70,13 @@ public String getBasePath() { @Override public void define(Service service) { service.get(BASE_PATH, this::getMappings, jsonTransformer); + service.put(BASE_PATH, this::addMappings); service.get(USER_MAPPING_PATH + ":" + USER, this::getUserMappings, jsonTransformer); } - private ImmutableListMultimap getMappings(Request request, Response response) { + private Map getMappings(Request request, Response response) { try { - return recipientRewriteTable.getAllMappings() - .entrySet() - .stream() - .flatMap(entry -> entry.getValue().asStream() - .map(mapping -> Pair.of( - entry.getKey().asString(), - MappingValueDTO.fromMapping(mapping)))) - .collect(ImmutableListMultimap.toImmutableListMultimap(Pair::getLeft, Pair::getRight)); + return recipientRewriteTable.getAllMappings(); } catch (RecipientRewriteTableException e) { throw ErrorResponder.builder() .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) @@ -86,13 +86,34 @@ private ImmutableListMultimap getMappings(Request reque } } - private List getUserMappings(Request request, Response response) throws RecipientRewriteTableException { - Username username = Username.of(request.params(USER).toLowerCase()); + public String addMappings(Request request, Response response) { + try { + Map mappings = jsonExtractor.parse(request.body()); + for (Map.Entry entry : mappings.entrySet()) { + for (Mapping mapping : entry.getValue()) { + recipientRewriteTable.addMapping(entry.getKey(), mapping); + } + } + return Responses.returnNoContent(response); + } catch (JsonExtractException e) { + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .message("error parsing mappings") + .cause(e) + .haltError(); + } catch (RecipientRewriteTableException e) { + throw ErrorResponder.builder() + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .message("error adding mappings") + .cause(e) + .haltError(); + } + } - return recipientRewriteTable.getStoredMappings(MappingSource.fromUser(username)) - .asStream() - .map(MappingValueDTO::fromMapping) - .collect(ImmutableList.toImmutableList()); + private Mappings getUserMappings(Request request, Response response) throws RecipientRewriteTableException { + Username username = Username.of(request.params(USER).toLowerCase()); + return recipientRewriteTable.getStoredMappings(MappingSource.fromUser(username)); } } - diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java index 9d0d9e32aeb..d1d7506c515 100644 --- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java +++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java @@ -52,8 +52,9 @@ import org.apache.james.user.memory.MemoryUsersRepository; import org.apache.james.webadmin.WebAdminServer; import org.apache.james.webadmin.WebAdminUtils; -import org.apache.james.webadmin.dto.MappingSourceModule; +import org.apache.james.webadmin.dto.MappingsModule; import org.apache.james.webadmin.utils.JsonTransformer; +import org.apache.james.webadmin.utils.JsonTransformerModule; import org.eclipse.jetty.http.HttpStatus; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -115,7 +116,7 @@ void setUp() throws Exception { domainList.addDomain(DOMAIN); domainList.addDomain(ALIAS_DOMAIN); domainList.addDomain(DOMAIN_MAPPING); - MappingSourceModule module = new MappingSourceModule(); + JsonTransformerModule module = new MappingsModule(); memoryRecipientRewriteTable.setDomainList(domainList); memoryRecipientRewriteTable.setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED); diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/ForwardRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/ForwardRoutesTest.java index 317997dfb0e..be2a489de06 100644 --- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/ForwardRoutesTest.java +++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/ForwardRoutesTest.java @@ -53,8 +53,9 @@ import org.apache.james.user.memory.MemoryUsersRepository; import org.apache.james.webadmin.WebAdminServer; import org.apache.james.webadmin.WebAdminUtils; -import org.apache.james.webadmin.dto.MappingSourceModule; +import org.apache.james.webadmin.dto.MappingsModule; import org.apache.james.webadmin.utils.JsonTransformer; +import org.apache.james.webadmin.utils.JsonTransformerModule; import org.eclipse.jetty.http.HttpStatus; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -115,7 +116,7 @@ void setUp() throws Exception { domainList.addDomain(DOMAIN_MAPPING); memoryRecipientRewriteTable.setDomainList(domainList); memoryRecipientRewriteTable.setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED); - MappingSourceModule mappingSourceModule = new MappingSourceModule(); + JsonTransformerModule mappingSourceModule = new MappingsModule(); usersRepository = MemoryUsersRepository.withVirtualHosting(domainList); diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java index 982c00c15d5..b6f78e4bed5 100644 --- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java +++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java @@ -54,8 +54,9 @@ import org.apache.james.user.memory.MemoryUsersRepository; import org.apache.james.webadmin.WebAdminServer; import org.apache.james.webadmin.WebAdminUtils; -import org.apache.james.webadmin.dto.MappingSourceModule; +import org.apache.james.webadmin.dto.MappingsModule; import org.apache.james.webadmin.utils.JsonTransformer; +import org.apache.james.webadmin.utils.JsonTransformerModule; import org.eclipse.jetty.http.HttpStatus; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -116,7 +117,7 @@ void setUp() throws Exception { memoryRecipientRewriteTable.setDomainList(domainList); memoryRecipientRewriteTable.setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED); usersRepository = MemoryUsersRepository.withVirtualHosting(domainList); - MappingSourceModule mappingSourceModule = new MappingSourceModule(); + JsonTransformerModule mappingSourceModule = new MappingsModule(); UserEntityValidator validator = UserEntityValidator.aggregate( new DefaultUserEntityValidator(usersRepository), new RecipientRewriteTableUserEntityValidator(memoryRecipientRewriteTable)); diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/MappingRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/MappingRoutesTest.java index 7cfee64db36..046deb63d55 100644 --- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/MappingRoutesTest.java +++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/MappingRoutesTest.java @@ -19,6 +19,7 @@ package org.apache.james.webadmin.routes; +import static io.restassured.RestAssured.given; import static io.restassured.RestAssured.when; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.hamcrest.CoreMatchers.is; @@ -38,6 +39,7 @@ import org.apache.james.user.memory.MemoryUsersRepository; import org.apache.james.webadmin.WebAdminServer; import org.apache.james.webadmin.WebAdminUtils; +import org.apache.james.webadmin.dto.MappingsModule; import org.apache.james.webadmin.utils.JsonTransformer; import org.eclipse.jetty.http.HttpStatus; import org.junit.jupiter.api.AfterEach; @@ -64,7 +66,7 @@ class MappingRoutesTest { @BeforeEach void setUp() throws Exception { - JsonTransformer jsonTransformer = new JsonTransformer(); + JsonTransformer jsonTransformer = new JsonTransformer(new MappingsModule()); recipientRewriteTable = new MemoryRecipientRewriteTable(); DNSService dnsService = mock(DNSService.class); MemoryDomainList domainList = new MemoryDomainList(dnsService); @@ -478,7 +480,52 @@ void getMappingsShouldReturnAllMappings() throws Exception { ); } - @Test + @Test + void addMappingsShouldAddMappings() throws Exception { + String importedJsonBody = "{" + + " \"alias@domain.tld\": [" + + " {" + + " \"type\": \"Alias\"," + + " \"mapping\": \"user@domain.tld\"" + + " }" + + " ]," + + " \"domain.mapping.tld\": [" + + " {" + + " \"type\": \"Domain\"," + + " \"mapping\": \"realdomain.tld\"" + + " }" + + " ]," + + " \"address@domain.tld\": [" + + " {" + + " \"type\": \"Address\"," + + " \"mapping\": \"user@domain.tld\"" + + " }" + + " ]" + + "}"; + + given() + .contentType(ContentType.JSON) + .body(importedJsonBody) + .when() + .put() + .then() + .statusCode(HttpStatus.NO_CONTENT_204); + + String jsonBody = when() + .get() + .then() + .contentType(ContentType.JSON) + .statusCode(HttpStatus.OK_200) + .extract() + .body() + .asString(); + + assertThatJson(jsonBody) + .isEqualTo(importedJsonBody); + } + + + @Test void getUserMappingsShouldReturnNotFoundByDefault() { when() .get("/user/") diff --git a/upgrade-instructions.md b/upgrade-instructions.md index 0c163ec6dfe..b471144aab0 100644 --- a/upgrade-instructions.md +++ b/upgrade-instructions.md @@ -34,6 +34,27 @@ Change list: - [Migrate RabbitMQ classic queues to version 2](#migrate-rabbitmq-classic-queues-to-version-2) - [JAMES-3946 White list removals](#james-3946-white-list-removals) - [JAMES-4052 Details in quota index](#james-4052-details-in-quota-index) + - [JAMES-1409 Change JPARecipientRewriteTable to store separate record per target address](#james-1409-change-jparecipientrewritetable-to-store-separate-record-per-target-address) + +### JAMES-1409 Change JPARecipientRewriteTable to store separate record per target address + +Date: 09/10/2024 + +JIRA: https://issues.apache.org/jira/browse/JAMES-1409 + +The JPARecipientRewriteTable was modified to store multiple mappings of a single source as separate database rows, +each with a single target address, instead of a single row with a long semicolon-delimited multi-value target address. +This solves both the limitation on the number of mappings (imposed by the column maximum length), and the broken query +by target address. + +For JPA users, the database schema update and data migration can be performed as follows: + +- On the old James server (before the version upgrade), export all mappings using the webadmin + interface (GET /mappings) and save the JSON result. +- Shut down the server. +- Drop the old table using any database administration tool (DROP TABLE JAMES_RECIPIENT_REWRITE). +- Upgrade and start the new James server. A new table will be created automatically with the new schema. +- Import the saved JSON mappings via the webadmin interface (PUT /mappings). ### JAMES-4052 Details in quota index and mailbox user