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

commit eb484d5663833e427fcc398f154a13f3862bc44c
Author: Benoit Tellier <[email protected]>
AuthorDate: Thu Oct 31 09:15:52 2019 +0700

    JAMES-2943 Throw upon detected domain removal
---
 .../api/AutoDetectedDomainRemovalException.java    | 28 +++++++++++
 .../domainlist/cassandra/CassandraDomainList.java  |  2 +-
 .../apache/james/domainlist/xml/XMLDomainList.java |  2 +-
 .../apache/james/domainlist/jpa/JPADomainList.java |  2 +-
 .../james/domainlist/lib/AbstractDomainList.java   | 58 ++++++++++++++++++----
 .../lib/AbstractDomainListPrivateMethodsTest.java  |  9 ++--
 .../james/domainlist/memory/MemoryDomainList.java  |  2 +-
 7 files changed, 82 insertions(+), 21 deletions(-)

diff --git 
a/server/data/data-api/src/main/java/org/apache/james/domainlist/api/AutoDetectedDomainRemovalException.java
 
b/server/data/data-api/src/main/java/org/apache/james/domainlist/api/AutoDetectedDomainRemovalException.java
new file mode 100644
index 0000000..85f129c
--- /dev/null
+++ 
b/server/data/data-api/src/main/java/org/apache/james/domainlist/api/AutoDetectedDomainRemovalException.java
@@ -0,0 +1,28 @@
+/****************************************************************
+ * 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.domainlist.api;
+
+import org.apache.james.core.Domain;
+
+public class AutoDetectedDomainRemovalException extends DomainListException {
+    public AutoDetectedDomainRemovalException(Domain domain) {
+        super(domain.asString() + " is autodetected and cannot be removed");
+    }
+}
diff --git 
a/server/data/data-cassandra/src/main/java/org/apache/james/domainlist/cassandra/CassandraDomainList.java
 
b/server/data/data-cassandra/src/main/java/org/apache/james/domainlist/cassandra/CassandraDomainList.java
index 4014e39..4eb0c6a 100644
--- 
a/server/data/data-cassandra/src/main/java/org/apache/james/domainlist/cassandra/CassandraDomainList.java
+++ 
b/server/data/data-cassandra/src/main/java/org/apache/james/domainlist/cassandra/CassandraDomainList.java
@@ -108,7 +108,7 @@ public class CassandraDomainList extends AbstractDomainList 
{
     }
 
     @Override
-    public void removeDomain(Domain domain) throws DomainListException {
+    public void doRemoveDomain(Domain domain) throws DomainListException {
         boolean executed = executor.executeReturnApplied(removeStatement.bind()
             .setString(DOMAIN, domain.asString()))
             .block();
diff --git 
a/server/data/data-file/src/main/java/org/apache/james/domainlist/xml/XMLDomainList.java
 
b/server/data/data-file/src/main/java/org/apache/james/domainlist/xml/XMLDomainList.java
index 27353c0..89c0724 100644
--- 
a/server/data/data-file/src/main/java/org/apache/james/domainlist/xml/XMLDomainList.java
+++ 
b/server/data/data-file/src/main/java/org/apache/james/domainlist/xml/XMLDomainList.java
@@ -72,7 +72,7 @@ public class XMLDomainList extends AbstractDomainList 
implements Configurable {
     }
 
     @Override
-    public void removeDomain(Domain domain) throws DomainListException {
+    public void doRemoveDomain(Domain domain) throws DomainListException {
         if (isConfigured) {
             throw new DomainListException("Read-Only DomainList 
implementation");
         }
diff --git 
a/server/data/data-jpa/src/main/java/org/apache/james/domainlist/jpa/JPADomainList.java
 
b/server/data/data-jpa/src/main/java/org/apache/james/domainlist/jpa/JPADomainList.java
index 2d74392..40cd999 100644
--- 
a/server/data/data-jpa/src/main/java/org/apache/james/domainlist/jpa/JPADomainList.java
+++ 
b/server/data/data-jpa/src/main/java/org/apache/james/domainlist/jpa/JPADomainList.java
@@ -143,7 +143,7 @@ public class JPADomainList extends AbstractDomainList {
     }
 
     @Override
-    public void removeDomain(Domain domain) throws DomainListException {
+    public void doRemoveDomain(Domain domain) throws DomainListException {
         EntityManager entityManager = 
entityManagerFactory.createEntityManager();
         final EntityTransaction transaction = entityManager.getTransaction();
         try {
diff --git 
a/server/data/data-library/src/main/java/org/apache/james/domainlist/lib/AbstractDomainList.java
 
b/server/data/data-library/src/main/java/org/apache/james/domainlist/lib/AbstractDomainList.java
index 30b4f75..a723829 100644
--- 
a/server/data/data-library/src/main/java/org/apache/james/domainlist/lib/AbstractDomainList.java
+++ 
b/server/data/data-library/src/main/java/org/apache/james/domainlist/lib/AbstractDomainList.java
@@ -30,6 +30,7 @@ import 
org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.configuration2.tree.ImmutableNode;
 import org.apache.james.core.Domain;
 import org.apache.james.dnsservice.api.DNSService;
+import org.apache.james.domainlist.api.AutoDetectedDomainRemovalException;
 import org.apache.james.domainlist.api.DomainList;
 import org.apache.james.domainlist.api.DomainListException;
 import org.apache.james.lifecycle.api.Configurable;
@@ -41,7 +42,9 @@ import com.github.steveash.guavate.Guavate;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Multimap;
 
 /**
  * All implementations of the DomainList interface should extends this abstract
@@ -51,6 +54,12 @@ public abstract class AbstractDomainList implements 
DomainList, Configurable {
 
     private static final Logger LOGGER = 
LoggerFactory.getLogger(AbstractDomainList.class);
 
+    enum DomainType {
+        Internal,
+        Detected,
+        DetectedIp
+    }
+
     public static final String CONFIGURE_AUTODETECT = "autodetect";
     public static final String CONFIGURE_AUTODETECT_IP = "autodetectIP";
     public static final String CONFIGURE_DEFAULT_DOMAIN = "defaultDomain";
@@ -155,17 +164,9 @@ public abstract class AbstractDomainList implements 
DomainList, Configurable {
 
     @Override
     public ImmutableList<Domain> getDomains() throws DomainListException {
-        List<Domain> domains = getDomainListInternal();
-        ImmutableList<Domain> detectedDomains = detectDomains();
-
-        ImmutableList<Domain> domainsWithoutIp = 
ImmutableList.<Domain>builder()
-            .addAll(domains)
-            .addAll(detectedDomains)
-            .build();
-        ImmutableSet<Domain> allDomains = ImmutableSet.<Domain>builder()
-            .addAll(domainsWithoutIp)
-            .addAll(detectIps(domainsWithoutIp))
-            .build();
+        ImmutableSet<Domain> allDomains = getDomainsWithType().values()
+            .stream()
+            .collect(Guavate.toImmutableSet());
 
         if (LOGGER.isDebugEnabled()) {
             for (Domain domain : allDomains) {
@@ -176,6 +177,23 @@ public abstract class AbstractDomainList implements 
DomainList, Configurable {
         return ImmutableList.copyOf(allDomains);
     }
 
+    private Multimap<DomainType, Domain> getDomainsWithType() throws 
DomainListException {
+        List<Domain> domains = getDomainListInternal();
+        ImmutableList<Domain> detectedDomains = detectDomains();
+
+        ImmutableList<Domain> domainsWithoutIp = 
ImmutableList.<Domain>builder()
+            .addAll(domains)
+            .addAll(detectedDomains)
+            .build();
+        ImmutableList<Domain> ips = detectIps(domainsWithoutIp);
+
+        return ImmutableMultimap.<DomainType, Domain>builder()
+            .putAll(DomainType.Internal, domains)
+            .putAll(DomainType.Detected, detectedDomains)
+            .putAll(DomainType.DetectedIp, ips)
+            .build();
+    }
+
     private ImmutableList<Domain> detectIps(Collection<Domain> domains) {
         if (autoDetectIP) {
             return getDomainsIpStream(domains, dns, LOGGER)
@@ -250,6 +268,22 @@ public abstract class AbstractDomainList implements 
DomainList, Configurable {
         this.autoDetectIP = autoDetectIP;
     }
 
+    @Override
+    public void removeDomain(Domain domain) throws DomainListException {
+        if (isAutoDetected(domain)) {
+            throw new AutoDetectedDomainRemovalException(domain);
+        }
+
+        doRemoveDomain(domain);
+    }
+
+    private boolean isAutoDetected(Domain domain) throws DomainListException {
+        Multimap<DomainType, Domain> domainsWithType = getDomainsWithType();
+
+        return domainsWithType.get(DomainType.Detected).contains(domain)
+            || domainsWithType.get(DomainType.DetectedIp).contains(domain);
+    }
+
     /**
      * Return domainList
      * 
@@ -259,4 +293,6 @@ public abstract class AbstractDomainList implements 
DomainList, Configurable {
 
     protected abstract boolean containsDomainInternal(Domain domain) throws 
DomainListException;
 
+    protected abstract void doRemoveDomain(Domain domain) throws 
DomainListException;
+
 }
diff --git 
a/server/data/data-library/src/test/java/org/apache/james/domainlist/lib/AbstractDomainListPrivateMethodsTest.java
 
b/server/data/data-library/src/test/java/org/apache/james/domainlist/lib/AbstractDomainListPrivateMethodsTest.java
index e8e8060..2100ec1 100644
--- 
a/server/data/data-library/src/test/java/org/apache/james/domainlist/lib/AbstractDomainListPrivateMethodsTest.java
+++ 
b/server/data/data-library/src/test/java/org/apache/james/domainlist/lib/AbstractDomainListPrivateMethodsTest.java
@@ -31,9 +31,8 @@ import java.util.List;
 
 import org.apache.james.core.Domain;
 import org.apache.james.dnsservice.api.DNSService;
-import org.apache.james.domainlist.api.DomainListException;
+import org.apache.james.domainlist.api.AutoDetectedDomainRemovalException;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.collect.ImmutableList;
@@ -320,7 +319,6 @@ public class AbstractDomainListPrivateMethodsTest {
         assertThat(domainList.containsDomain(Domain.of(envDomain))).isTrue();
     }
 
-    @Ignore("JAMES-2943 Removing an auto-detected domain leads to a noop")
     @Test
     public void removeDomainShouldThrowWhenRemovingAutoDetectedDomains() 
throws Exception {
         domainList.configure(DomainListConfiguration.builder()
@@ -332,10 +330,9 @@ public class AbstractDomainListPrivateMethodsTest {
         
when(dnsService.getHostName(any(InetAddress.class))).thenReturn(detected);
 
         assertThatThrownBy(() -> domainList.removeDomain(Domain.of(detected)))
-            .isInstanceOf(DomainListException.class);
+            .isInstanceOf(AutoDetectedDomainRemovalException.class);
     }
 
-    @Ignore("JAMES-2943 Removing an auto-detected ip leads to a noop")
     @Test
     public void removeDomainShouldThrowWhenRemovingAutoDetectedIps() throws 
Exception {
         String detected = "detected.tld";
@@ -347,7 +344,7 @@ public class AbstractDomainListPrivateMethodsTest {
         
when(dnsService.getAllByName(detected)).thenReturn(ImmutableList.of(detectedAddress));
 
         assertThatThrownBy(() -> 
domainList.removeDomain(Domain.of(detectedIp)))
-            .isInstanceOf(DomainListException.class);
+            .isInstanceOf(AutoDetectedDomainRemovalException.class);
     }
 
     @Test
diff --git 
a/server/data/data-memory/src/main/java/org/apache/james/domainlist/memory/MemoryDomainList.java
 
b/server/data/data-memory/src/main/java/org/apache/james/domainlist/memory/MemoryDomainList.java
index c6d89c3..1c8d679 100644
--- 
a/server/data/data-memory/src/main/java/org/apache/james/domainlist/memory/MemoryDomainList.java
+++ 
b/server/data/data-memory/src/main/java/org/apache/james/domainlist/memory/MemoryDomainList.java
@@ -60,7 +60,7 @@ public class MemoryDomainList extends AbstractDomainList {
     }
 
     @Override
-    public void removeDomain(Domain domain) throws DomainListException {
+    public void doRemoveDomain(Domain domain) throws DomainListException {
         if (!domains.remove(domain)) {
             throw new DomainListException(domain.name() + " was not found");
         }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to