This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new 0f484fb222 JAMES-4079 Fix RecipientRewriteTable adds duplicate mapping
(#2449)
0f484fb222 is described below
commit 0f484fb22249750cc2f9ea04ca2c248435a66fbe
Author: amichair <amich...@amichais.net>
AuthorDate: Mon Oct 14 10:03:53 2024 +0300
JAMES-4079 Fix RecipientRewriteTable adds duplicate mapping (#2449)
---
.../cassandra/CassandraRecipientRewriteTable.java | 8 --
.../james/rrt/file/XMLRecipientRewriteTable.java | 9 --
.../rrt/file/XMLRecipientRewriteTableTest.java | 5 ++
.../james/rrt/jpa/JPARecipientRewriteTable.java | 13 ---
.../rrt/lib/AbstractRecipientRewriteTable.java | 8 +-
.../rrt/lib/RecipientRewriteTableContract.java | 14 +++
.../rrt/memory/MemoryRecipientRewriteTable.java | 99 ++++------------------
7 files changed, 44 insertions(+), 112 deletions(-)
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 6eda5a3213..390da1f6ca 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
@@ -24,7 +24,6 @@ import java.util.stream.Stream;
import jakarta.inject.Inject;
import org.apache.commons.lang3.tuple.Pair;
-import org.apache.james.core.Domain;
import org.apache.james.rrt.api.RecipientRewriteTableException;
import org.apache.james.rrt.lib.AbstractRecipientRewriteTable;
import org.apache.james.rrt.lib.Mapping;
@@ -79,13 +78,6 @@ public class CassandraRecipientRewriteTable extends
AbstractRecipientRewriteTabl
.block();
}
- @Override
- protected Mappings mapAddress(String user, Domain domain) {
- return
cassandraRecipientRewriteTableDAO.retrieveMappings(MappingSource.fromUser(user,
domain)).blockOptional()
- .or(() ->
cassandraRecipientRewriteTableDAO.retrieveMappings(MappingSource.fromDomain(domain)).blockOptional())
- .orElse(MappingsImpl.empty());
- }
-
@Override
public Stream<MappingSource> listSources(Mapping mapping) throws
RecipientRewriteTableException {
Preconditions.checkArgument(listSourcesSupportedType.contains(mapping.getType()),
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 fe815e72fd..f8c986120c 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
@@ -25,7 +25,6 @@ import java.util.Optional;
import org.apache.commons.configuration2.HierarchicalConfiguration;
import org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.commons.configuration2.tree.ImmutableNode;
-import org.apache.james.core.Domain;
import org.apache.james.rrt.api.RecipientRewriteTableException;
import org.apache.james.rrt.lib.AbstractRecipientRewriteTable;
import org.apache.james.rrt.lib.Mapping;
@@ -60,14 +59,6 @@ public class XMLRecipientRewriteTable extends
AbstractRecipientRewriteTable {
}
}
- @Override
- protected Mappings mapAddress(String user, Domain domain) {
- return Optional.ofNullable(mappings)
- .map(mappings -> RecipientRewriteTableUtil.getTargetString(user,
domain, mappings))
- .map(MappingsImpl::fromRawString)
- .orElse(MappingsImpl.empty());
- }
-
@Override
public Mappings getStoredMappings(MappingSource source) {
return Optional.ofNullable(mappings)
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 01c6aee368..461441225d 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
@@ -78,6 +78,11 @@ class XMLRecipientRewriteTableTest implements
RecipientRewriteTableContract {
public void testStoreAndGetMappings() {
}
+ @Test
+ @Disabled("XMLRecipientRewriteTable is read only")
+ public void testStoreDuplicateMapping() {
+ }
+
@Test
@Disabled("XMLRecipientRewriteTable is read only")
public void testStoreAndRetrieveRegexMapping() {
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 9ba6b3500d..2e3a28c6b1 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
@@ -113,19 +113,6 @@ public class JPARecipientRewriteTable extends
AbstractRecipientRewriteTable {
}
}
- @Override
- protected Mappings mapAddress(String user, Domain domain) throws
RecipientRewriteTableException {
- Mappings userDomainMapping =
getStoredMappings(MappingSource.fromUser(user, domain));
- if (userDomainMapping != null && !userDomainMapping.isEmpty()) {
- return userDomainMapping;
- }
- Mappings domainMapping =
getStoredMappings(MappingSource.fromDomain(domain));
- if (domainMapping != null && !domainMapping.isEmpty()) {
- return domainMapping;
- }
- return MappingsImpl.empty();
- }
-
@Override
@SuppressWarnings("unchecked")
public Mappings getStoredMappings(MappingSource source) throws
RecipientRewriteTableException {
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 020f4379d4..feb85ca65c 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
@@ -406,7 +406,13 @@ public abstract class AbstractRecipientRewriteTable
implements RecipientRewriteT
* It must never return null but throw RecipientRewriteTableException on
errors and return an empty Mappings
* object if no mapping is found.
*/
- protected abstract Mappings mapAddress(String user, Domain domain) throws
RecipientRewriteTableException;
+ protected Mappings mapAddress(String user, Domain domain) throws
RecipientRewriteTableException {
+ Mappings mappings = getStoredMappings(MappingSource.fromUser(user,
domain));
+ if (mappings.isEmpty()) {
+ mappings = getStoredMappings(MappingSource.fromDomain(domain));
+ }
+ return mappings;
+ }
private void checkDomainMappingSourceIsManaged(MappingSource source)
throws RecipientRewriteTableException {
Optional<Domain> notManagedSourceDomain = source.availableDomain()
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 1acc5cab38..edff932426 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
@@ -104,6 +104,20 @@ public interface RecipientRewriteTableContract {
assertThat(virtualUserTable().getResolvedMappings("prefix_abc",
domain)).isNotEmpty();
}
+ @Test
+ default void testStoreDuplicateMapping() throws Exception {
+ Domain domain = Domain.of("james");
+ MappingSource source = MappingSource.fromUser(USER, domain);
+
+ Mapping mapping = Mapping.group(ADDRESS);
+ Mapping mapping2 = Mapping.group(ADDRESS);
+
+ virtualUserTable().addMapping(source, mapping);
+ virtualUserTable().addMapping(source, mapping2);
+
+ assertThat(virtualUserTable().mapAddress(USER,
domain).size()).isEqualTo(1);
+ }
+
@Test
default void testQuotedLocalPart() {
assertThatCode(() -> virtualUserTable().getResolvedMappings("\"a@b\"",
Domain.of("test"))).doesNotThrowAnyException();
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 68996362e0..b5a57fbdfd 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
@@ -19,113 +19,50 @@
package org.apache.james.rrt.memory;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
import java.util.Map;
-import java.util.Optional;
-import java.util.stream.Stream;
+import java.util.Set;
+import java.util.stream.Collectors;
-import org.apache.commons.lang3.tuple.Pair;
-import org.apache.james.core.Domain;
-import org.apache.james.core.Username;
import org.apache.james.rrt.lib.AbstractRecipientRewriteTable;
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 com.google.common.base.Objects;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Multimaps;
-
public class MemoryRecipientRewriteTable extends AbstractRecipientRewriteTable
{
- private static class InMemoryMappingEntry {
- private final MappingSource source;
- private final Mapping mapping;
-
- public InMemoryMappingEntry(MappingSource source, Mapping mapping) {
- this.source = source;
- this.mapping = mapping;
- }
-
- public MappingSource getSource() {
- return source;
- }
-
- public Mapping getMapping() {
- return mapping;
- }
-
- @Override
- public boolean equals(Object o) {
- if (o == null || this.getClass() != o.getClass()) {
- return false;
- }
-
- InMemoryMappingEntry that = (InMemoryMappingEntry) o;
-
- return Objects.equal(this.source, that.source)
- && Objects.equal(this.mapping, that.mapping);
- }
-
- @Override
- public int hashCode() {
- return Objects.hashCode(source, mapping);
- }
- }
+ private final Map<MappingSource, Set<Mapping>> table = new HashMap<>();
- private final List<InMemoryMappingEntry> mappingEntries;
-
- public MemoryRecipientRewriteTable() {
- mappingEntries = new ArrayList<>();
+ private Mappings toMappings(Set<Mapping> mappings) {
+ return mappings == null ? MappingsImpl.empty() :
MappingsImpl.fromMappings(mappings.stream());
}
@Override
public void addMapping(MappingSource source, Mapping mapping) {
- mappingEntries.add(new InMemoryMappingEntry(source, mapping));
+ table.computeIfAbsent(source, s -> new LinkedHashSet<>()).add(mapping);
}
@Override
public void removeMapping(MappingSource source, Mapping mapping) {
- mappingEntries.remove(new InMemoryMappingEntry(source, mapping));
+ Set<Mapping> mappings = table.get(source);
+ if (mappings != null) {
+ mappings.remove(mapping);
+ if (mappings.isEmpty()) {
+ table.remove(source);
+ }
+ }
}
@Override
public Mappings getStoredMappings(MappingSource mappingSource) {
- return retrieveMappings(mappingSource)
- .orElse(MappingsImpl.empty());
- }
-
- @Override
- protected Mappings mapAddress(String user, Domain domain) {
- return
retrieveMappings(MappingSource.fromUser(Username.fromLocalPartWithDomain(user,
domain)))
- .or(() -> retrieveMappings(MappingSource.fromDomain(domain)))
- .orElse(MappingsImpl.empty());
+ return toMappings(table.get(mappingSource));
}
@Override
public Map<MappingSource, Mappings> getAllMappings() {
- return Multimaps.index(mappingEntries, InMemoryMappingEntry::getSource)
- .asMap()
- .entrySet()
- .stream()
- .map(entry -> Pair.of(entry.getKey(),
toMappings(entry.getValue())))
- .collect(ImmutableMap.toImmutableMap(Pair::getKey,
Pair::getValue));
- }
-
- private MappingsImpl toMappings(Collection<InMemoryMappingEntry>
inMemoryMappingEntries) {
- return MappingsImpl.fromMappings(inMemoryMappingEntries
- .stream()
- .map(InMemoryMappingEntry::getMapping));
+ return table.entrySet().stream()
+ .collect(Collectors.toMap(e -> e.getKey(), e ->
toMappings(e.getValue())));
}
-
- private Optional<Mappings> retrieveMappings(MappingSource mappingSource) {
- Stream<Mapping> userEntries = mappingEntries.stream()
- .filter(mappingEntry -> mappingEntry.source.equals(mappingSource))
- .map(InMemoryMappingEntry::getMapping);
- return MappingsImpl.fromMappings(userEntries).toOptional();
- }
-
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
