You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2024/08/01 04:39:48 UTC
(james-project) 01/02: JAMES-4044 Add a cache for LDAP attribute matchers
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit f3dc00b3492cf9f5a87f44590289314325719ec6
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Sat Jul 27 08:38:20 2024 +0200
JAMES-4044 Add a cache for LDAP attribute matchers
---
docs/modules/servers/partials/LDAPMatchers.adoc | 14 ++-
.../james/transport/matchers/CacheSettings.java | 74 +++++++++++++
.../james/transport/matchers/HasLDAPAttribute.java | 37 ++++++-
.../transport/matchers/SenderHasLDAPAttribute.java | 36 ++++++-
.../transport/matchers/CacheSettingsTest.java | 115 +++++++++++++++++++++
.../transport/matchers/HasLDAPAttributeTest.java | 40 +++++++
.../matchers/SenderHasLDAPAttributeTest.java | 44 ++++++++
7 files changed, 355 insertions(+), 5 deletions(-)
diff --git a/docs/modules/servers/partials/LDAPMatchers.adoc b/docs/modules/servers/partials/LDAPMatchers.adoc
index 26b6bed7ad..418738ee24 100644
--- a/docs/modules/servers/partials/LDAPMatchers.adoc
+++ b/docs/modules/servers/partials/LDAPMatchers.adoc
@@ -64,7 +64,6 @@ or
</mailet>
....
-
For matching recipients against a specific LDAP group:
....xml
@@ -80,3 +79,16 @@ For matching a sender against a specific LDAP group:
</mailet>
....
+
+=== Cache settings
+
+A cache can optionally be turned on in order to reduce LDAP calls for `SenderHasLDAPAttribute` and `HasLDAPAttribute`:
+
+....xml
+<!-- Matches sender that have the following attribute regardless of the actual value-->
+<mailet matcher="SenderHasLDAPAttribute=description?cacheEnabled=true&cacheSize=1000&cacheDuration=1hour" class="Null">
+
+</mailet>
+....
+
+The defaults are cache up to 10_000 entries for 1 day.
\ No newline at end of file
diff --git a/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/CacheSettings.java b/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/CacheSettings.java
new file mode 100644
index 0000000000..514f60ff41
--- /dev/null
+++ b/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/CacheSettings.java
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.matchers;
+
+import java.time.Duration;
+import java.util.Locale;
+import java.util.Optional;
+
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.james.util.DurationParser;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.collect.ImmutableMap;
+
+public record CacheSettings(Duration duration, int size) {
+ public static Optional<CacheSettings> parse(String condition) {
+ if (!condition.contains("?")) {
+ return Optional.empty();
+ }
+ String cacheSettings = condition.substring(condition.indexOf('?') + 1);
+ ImmutableMap<String, String> cacheSettingMap = Splitter.on('&').trimResults().omitEmptyStrings().splitToStream(cacheSettings)
+ .map(CacheSettings::asPair)
+ .collect(ImmutableMap.toImmutableMap(Pair::getKey, Pair::getValue));
+ boolean enabled = Optional.ofNullable(cacheSettingMap.get("cacheenabled"))
+ .map(Boolean::parseBoolean)
+ .orElse(true);
+ int size = Optional.ofNullable(cacheSettingMap.get("cachesize"))
+ .map(Integer::parseInt)
+ .orElse(10_000);
+ Duration duration = Optional.ofNullable(cacheSettingMap.get("cacheduration"))
+ .map(DurationParser::parse)
+ .orElse(Duration.ofDays(1));
+
+ Preconditions.checkArgument(size > 0, "size must be an integer greater than 0");
+
+ if (!enabled) {
+ return Optional.empty();
+ }
+ return Optional.of(new CacheSettings(duration, size));
+ }
+
+ private static Pair<String, String> asPair(String condition) {
+ Preconditions.checkArgument(condition.contains("="), "Each pair of the cache setting must be of the form key=value. Got " + condition);
+
+ int separatorPosition = condition.indexOf('=');
+ String key = condition.substring(0, separatorPosition).toLowerCase(Locale.US);
+ String value = condition.substring(separatorPosition + 1);
+ return Pair.of(key, value);
+ }
+
+ Cache<String, Boolean> createAssociatedCache() {
+ return CacheBuilder.newBuilder().expireAfterWrite(duration).maximumSize(size).build();
+ }
+}
diff --git a/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/HasLDAPAttribute.java b/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/HasLDAPAttribute.java
index cecb20a681..79c0a748c3 100644
--- a/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/HasLDAPAttribute.java
+++ b/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/HasLDAPAttribute.java
@@ -33,6 +33,7 @@ import org.apache.mailet.Mail;
import org.apache.mailet.base.GenericMatcher;
import com.github.fge.lambdas.Throwing;
+import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.unboundid.ldap.sdk.Attribute;
@@ -67,6 +68,18 @@ import com.unboundid.ldap.sdk.SearchScope;
*
* </mailet>
* </code></pre>
+ *
+ * A cache can optionally be turned on in order to reduce LDAP calls:
+ *
+ *
+ * <pre><code>
+ * <!-- Matches recipients that have the following attribute with the specified value-->
+ * <mailet matcher="HasLDAPAttribute=description:blocked?cacheEnabled=true&cacheSize=1000&cacheDuration=1hour" class="Null">
+ *
+ * </mailet>
+ * </code></pre>
+ *
+ * The defaults are cache up to 10_000 entries for 1 day.
*/
public class HasLDAPAttribute extends GenericMatcher {
private final LDAPConnectionPool ldapConnectionPool;
@@ -76,6 +89,7 @@ public class HasLDAPAttribute extends GenericMatcher {
private String attributeName;
private Optional<String> attributeValue;
private String[] attributes;
+ private Optional<Cache<String, Boolean>> cache;
@Inject
public HasLDAPAttribute(LdapRepositoryConfiguration configuration) throws LDAPException {
@@ -89,7 +103,7 @@ public class HasLDAPAttribute extends GenericMatcher {
@Override
public void init() throws MessagingException {
- String condition = getCondition().trim();
+ String condition = removeCacheSettings();
int commaPosition = condition.indexOf(':');
if (commaPosition == -1) {
@@ -107,6 +121,17 @@ public class HasLDAPAttribute extends GenericMatcher {
.add(configuration.getReturnedAttributes())
.add(attributeName)
.build().toArray(String[]::new);
+
+ cache = CacheSettings.parse(getCondition()).map(CacheSettings::createAssociatedCache);
+ }
+
+ private String removeCacheSettings() {
+ int conditionEnd = getCondition().indexOf('?');
+ if (conditionEnd == -1) {
+ return getCondition().trim();
+ } else {
+ return getCondition().substring(0, conditionEnd).trim();
+ }
}
@Override
@@ -118,15 +143,23 @@ public class HasLDAPAttribute extends GenericMatcher {
}
private boolean hasAttribute(MailAddress rcpt) {
+ Optional<Boolean> cacheAnswer = cache.flatMap(c -> Optional.ofNullable(c.getIfPresent(rcpt.asString())));
+ if (cacheAnswer.isPresent()) {
+ return cacheAnswer.get();
+ }
try {
SearchResult searchResult = ldapConnectionPool.search(userBase(rcpt),
SearchScope.SUB,
createFilter(rcpt.asString(), configuration.getUserIdAttribute()),
attributes);
- return searchResult.getSearchEntries()
+ boolean answer = searchResult.getSearchEntries()
.stream()
.anyMatch(this::hasAttribute);
+
+ cache.ifPresent(c -> c.put(rcpt.asString(), answer));
+
+ return answer;
} catch (LDAPSearchException e) {
throw new RuntimeException("Failed searching LDAP", e);
}
diff --git a/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/SenderHasLDAPAttribute.java b/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/SenderHasLDAPAttribute.java
index 943dc65d0b..4058723224 100644
--- a/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/SenderHasLDAPAttribute.java
+++ b/server/mailet/ldap/src/main/java/org/apache/james/transport/matchers/SenderHasLDAPAttribute.java
@@ -33,6 +33,7 @@ import org.apache.mailet.Mail;
import org.apache.mailet.base.GenericMatcher;
import com.github.fge.lambdas.Throwing;
+import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.unboundid.ldap.sdk.Attribute;
@@ -67,6 +68,18 @@ import com.unboundid.ldap.sdk.SearchScope;
*
* </mailet>
* </code></pre>
+ *
+ * A cache can optionally be turned on in order to reduce LDAP calls:
+ *
+ *
+ * <pre><code>
+ * <!-- Matches recipients that have the following attribute with the specified value-->
+ * <mailet matcher="SenderHasLDAPAttribute=description:blocked?cacheEnabled=true&cacheSize=1000&cacheDuration=1hour" class="Null">
+ *
+ * </mailet>
+ * </code></pre>
+ *
+ * The defaults are cache up to 10_000 entries for 1 day.
*/
public class SenderHasLDAPAttribute extends GenericMatcher {
private final LDAPConnectionPool ldapConnectionPool;
@@ -76,6 +89,7 @@ public class SenderHasLDAPAttribute extends GenericMatcher {
private String attributeName;
private Optional<String> attributeValue;
private String[] attributes;
+ private Optional<Cache<String, Boolean>> cache;
@Inject
public SenderHasLDAPAttribute(LDAPConnectionPool ldapConnectionPool, LdapRepositoryConfiguration configuration) {
@@ -92,7 +106,7 @@ public class SenderHasLDAPAttribute extends GenericMatcher {
@Override
public void init() throws MessagingException {
- String condition = getCondition().trim();
+ String condition = removeCacheSettings();
int commaPosition = condition.indexOf(':');
if (commaPosition == -1) {
@@ -110,6 +124,17 @@ public class SenderHasLDAPAttribute extends GenericMatcher {
.add(configuration.getReturnedAttributes())
.add(attributeName)
.build().toArray(String[]::new);
+
+ cache = CacheSettings.parse(getCondition()).map(CacheSettings::createAssociatedCache);
+ }
+
+ private String removeCacheSettings() {
+ int conditionEnd = getCondition().indexOf('?');
+ if (conditionEnd == -1) {
+ return getCondition().trim();
+ } else {
+ return getCondition().substring(0, conditionEnd).trim();
+ }
}
@Override
@@ -124,15 +149,22 @@ public class SenderHasLDAPAttribute extends GenericMatcher {
}
private boolean hasAttribute(MailAddress rcpt) {
+ Optional<Boolean> cacheAnswer = cache.flatMap(c -> Optional.ofNullable(c.getIfPresent(rcpt.asString())));
+ if (cacheAnswer.isPresent()) {
+ return cacheAnswer.get();
+ }
try {
SearchResult searchResult = ldapConnectionPool.search(userBase(rcpt),
SearchScope.SUB,
createFilter(rcpt.asString(), configuration.getUserIdAttribute()),
attributes);
- return searchResult.getSearchEntries().stream()
+ boolean answer = searchResult.getSearchEntries().stream()
.anyMatch(this::hasAttribute);
+ cache.ifPresent(c -> c.put(rcpt.asString(), answer));
+
+ return answer;
} catch (LDAPSearchException e) {
throw new RuntimeException("Failed searching LDAP", e);
}
diff --git a/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/CacheSettingsTest.java b/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/CacheSettingsTest.java
new file mode 100644
index 0000000000..a4fbd21b72
--- /dev/null
+++ b/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/CacheSettingsTest.java
@@ -0,0 +1,115 @@
+/****************************************************************
+ * 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.matchers;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.Test;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
+
+class CacheSettingsTest {
+ @Test
+ void shouldMatchBeanContract() {
+ EqualsVerifier.forClass(CacheSettings.class)
+ .verify();
+ }
+
+ @Test
+ void shouldReturnEmptyWhenEmpty() {
+ assertThat(CacheSettings.parse(""))
+ .isEmpty();
+ }
+
+ @Test
+ void shouldReturnEmptyWhenNoQuestionMark() {
+ assertThat(CacheSettings.parse("abcdef"))
+ .isEmpty();
+ }
+
+ @Test
+ void shouldReturnEmptyWhenCacheDisabled() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnabled=false"))
+ .isEmpty();
+ }
+
+ @Test
+ void shouldBeTolerantRegardingCaseInKeys() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnaBLed=false"))
+ .isEmpty();
+ }
+
+ @Test
+ void shouldReturnDefaultWhenNoSettings() {
+ assertThat(CacheSettings.parse("abcdef?"))
+ .contains(new CacheSettings(Duration.ofDays(1), 10_000));
+ }
+
+ @Test
+ void shouldReturnDefaultWhenEnabled() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnabled=true"))
+ .contains(new CacheSettings(Duration.ofDays(1), 10_000));
+ }
+
+ @Test
+ void shouldTrimKeys() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnabled=true "))
+ .contains(new CacheSettings(Duration.ofDays(1), 10_000));
+ }
+
+ @Test
+ void shouldIgnoreEmptyKeys() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnabled=true&"))
+ .contains(new CacheSettings(Duration.ofDays(1), 10_000));
+ }
+
+ @Test
+ void shouldIgnoreUnknownSettings() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnabled=true&unrelated=ignored"))
+ .contains(new CacheSettings(Duration.ofDays(1), 10_000));
+ }
+
+ @Test
+ void shouldAcceptCustomSize() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnabled=true&cacheSize=1234"))
+ .contains(new CacheSettings(Duration.ofDays(1), 1234));
+ }
+
+ @Test
+ void shouldAcceptCustomDuration() {
+ assertThat(CacheSettings.parse("abcdef?cacheEnabled=true&cacheDuration=1234minutes"))
+ .contains(new CacheSettings(Duration.ofMinutes(1234), 10_000));
+ }
+
+ @Test
+ void shouldRejectBadlyFormattedKey() {
+ assertThatThrownBy(() -> CacheSettings.parse("abcdef?noQuestionMark"))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ void shouldRejectNegativeSize() {
+ assertThatThrownBy(() -> CacheSettings.parse("abcdef?cacheSize=-1"))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+}
\ No newline at end of file
diff --git a/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/HasLDAPAttributeTest.java b/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/HasLDAPAttributeTest.java
index 7be38f426a..d3993281d0 100644
--- a/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/HasLDAPAttributeTest.java
+++ b/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/HasLDAPAttributeTest.java
@@ -76,6 +76,46 @@ class HasLDAPAttributeTest {
assertThat(matched).containsOnly(recipient);
}
+ @Test
+ void shouldMatchWhenCached() throws Exception {
+ HasLDAPAttribute testee = new HasLDAPAttribute(LdapRepositoryConfiguration.from(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer)));
+ FakeMatcherConfig matcherConfig = FakeMatcherConfig.builder()
+ .matcherName("HasLDAPAttribute")
+ .condition("description:abcdef?cacheEnabled=true")
+ .build();
+ testee.init(matcherConfig);
+
+ MailAddress recipient = new MailAddress("james-user@james.org");
+ FakeMail mail = FakeMail.builder()
+ .name("default-id")
+ .recipient(recipient)
+ .build();
+ testee.match(mail);
+ Collection<MailAddress> matched = testee.match(mail);
+
+ assertThat(matched).containsOnly(recipient);
+ }
+
+ @Test
+ void shouldAllowNoMatchWhenCached() throws Exception {
+ HasLDAPAttribute testee = new HasLDAPAttribute(LdapRepositoryConfiguration.from(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer)));
+ FakeMatcherConfig matcherConfig = FakeMatcherConfig.builder()
+ .matcherName("HasLDAPAttribute")
+ .condition("description:abcdef?cacheEnabled=true")
+ .build();
+ testee.init(matcherConfig);
+
+ MailAddress recipient = new MailAddress("james-user2@james.org");
+ FakeMail mail = FakeMail.builder()
+ .name("default-id")
+ .recipient(recipient)
+ .build();
+ testee.match(mail);
+ Collection<MailAddress> matched = testee.match(mail);
+
+ assertThat(matched).isEmpty();
+ }
+
@Test
void shouldThrowWhenPrefixedWithDelimiter() throws Exception {
HasLDAPAttribute testee = new HasLDAPAttribute(LdapRepositoryConfiguration.from(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer)));
diff --git a/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/SenderHasLDAPAttributeTest.java b/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/SenderHasLDAPAttributeTest.java
index cbf1cfc500..b151922fee 100644
--- a/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/SenderHasLDAPAttributeTest.java
+++ b/server/mailet/ldap/src/test/java/org/apache/james/transport/matchers/SenderHasLDAPAttributeTest.java
@@ -103,6 +103,50 @@ class SenderHasLDAPAttributeTest {
assertThat(matched).containsOnly(recipient);
}
+ @Test
+ void shouldMatchWhenCached() throws Exception {
+ SenderHasLDAPAttribute testee = new SenderHasLDAPAttribute(LdapRepositoryConfiguration.from(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer)));
+ FakeMatcherConfig matcherConfig = FakeMatcherConfig.builder()
+ .matcherName("SenderHasLDAPAttribute")
+ .condition("description?cacheEnabled=true")
+ .build();
+ testee.init(matcherConfig);
+
+ MailAddress sender = new MailAddress("james-user@james.org");
+ MailAddress recipient = new MailAddress("recipient@james.org");
+ FakeMail mail = FakeMail.builder()
+ .name("default-id")
+ .sender(sender)
+ .recipient(recipient)
+ .build();
+ testee.match(mail);
+ Collection<MailAddress> matched = testee.match(mail);
+
+ assertThat(matched).containsOnly(recipient);
+ }
+
+ @Test
+ void shouldAllowNoMatchWhenCached() throws Exception {
+ SenderHasLDAPAttribute testee = new SenderHasLDAPAttribute(LdapRepositoryConfiguration.from(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer)));
+ FakeMatcherConfig matcherConfig = FakeMatcherConfig.builder()
+ .matcherName("SenderHasLDAPAttribute")
+ .condition("description?cacheEnabled=true")
+ .build();
+ testee.init(matcherConfig);
+
+ MailAddress sender = new MailAddress("james-user2@james.org");
+ MailAddress recipient = new MailAddress("recipient@james.org");
+ FakeMail mail = FakeMail.builder()
+ .name("default-id")
+ .sender(sender)
+ .recipient(recipient)
+ .build();
+ testee.match(mail);
+ Collection<MailAddress> matched = testee.match(mail);
+
+ assertThat(matched).isEmpty();
+ }
+
@Test
void shouldNotReturnRecipientWhenDoesNotHaveAttribute() throws Exception {
SenderHasLDAPAttribute testee = new SenderHasLDAPAttribute(LdapRepositoryConfiguration.from(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer)));
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org