You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/03/20 12:15:48 UTC

[james-project] 05/08: JAMES-3113 LocalResources: propagate exceptions

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 262d064003c1baf4a7343c4ac0928c952e990214
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Wed Mar 18 10:37:19 2020 +0700

    JAMES-3113 LocalResources: propagate exceptions
    
    Avoid taking wrong business decisions upon technical exceptions
---
 .../james/mailetcontainer/impl/LocalResources.java |  15 ++-
 .../impl/JamesMailetContextTest.java               | 114 +++++++++++++++++++--
 2 files changed, 110 insertions(+), 19 deletions(-)

diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/LocalResources.java b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/LocalResources.java
index 509f80b..5ee133f 100644
--- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/LocalResources.java
+++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/LocalResources.java
@@ -58,8 +58,7 @@ class LocalResources {
         try {
             return domains.containsDomain(domain);
         } catch (DomainListException e) {
-            LOGGER.error("Unable to retrieve domains", e);
-            return false;
+            throw new RuntimeException("Unable to retrieve domains", e);
         }
     }
 
@@ -71,11 +70,9 @@ class LocalResources {
             MailAddress mailAddress = Username.of(name).withDefaultDomain(domains.getDefaultDomain()).asMailAddress();
             return isLocalEmail(mailAddress);
         } catch (DomainListException e) {
-            LOGGER.error("Unable to access DomainList", e);
-            return false;
+            throw new RuntimeException("Unable to retrieve domains", e);
         } catch (ParseException e) {
-            LOGGER.info("Error checking isLocalUser for user {}", name, e);
-            return false;
+            throw new RuntimeException("Unable to parse mail address", e);
         }
     }
 
@@ -87,8 +84,10 @@ class LocalResources {
             try {
                 return isLocaluser(mailAddress)
                     || isLocalAlias(mailAddress);
-            } catch (UsersRepositoryException | RecipientRewriteTable.ErrorMappingException | RecipientRewriteTableException e) {
-                LOGGER.error("Unable to access UsersRepository", e);
+            } catch (UsersRepositoryException e) {
+                throw new RuntimeException("Unable to retrieve users", e);
+            } catch (RecipientRewriteTable.ErrorMappingException | RecipientRewriteTableException e) {
+                throw new RuntimeException("Unable to retrieve RRTs", e);
             }
         }
         return false;
diff --git a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java
index 9cbf559..ea2320b 100644
--- a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java
+++ b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java
@@ -20,7 +20,11 @@
 package org.apache.james.mailetcontainer.impl;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.verifyZeroInteractions;
@@ -36,12 +40,15 @@ import org.apache.james.core.MailAddress;
 import org.apache.james.core.Username;
 import org.apache.james.core.builder.MimeMessageBuilder;
 import org.apache.james.dnsservice.api.DNSService;
+import org.apache.james.domainlist.api.DomainListException;
 import org.apache.james.domainlist.lib.DomainListConfiguration;
 import org.apache.james.domainlist.memory.MemoryDomainList;
 import org.apache.james.queue.api.MailQueue;
 import org.apache.james.queue.api.MailQueueFactory;
+import org.apache.james.rrt.api.RecipientRewriteTableException;
 import org.apache.james.rrt.memory.MemoryRecipientRewriteTable;
 import org.apache.james.server.core.MailImpl;
+import org.apache.james.user.api.UsersRepositoryException;
 import org.apache.james.user.memory.MemoryUsersRepository;
 import org.apache.james.util.MimeMessageUtil;
 import org.apache.mailet.Mail;
@@ -56,11 +63,11 @@ import org.mockito.ArgumentCaptor;
 import com.google.common.collect.ImmutableList;
 
 public class JamesMailetContextTest {
-    public static final Domain DOMAIN_COM = Domain.of("domain.com");
-    public static final String USERNAME = "user";
-    public static final Username USERMAIL = Username.of(USERNAME + "@" + DOMAIN_COM.name());
-    public static final String PASSWORD = "password";
-    public static final DNSService DNS_SERVICE = null;
+    private static final Domain DOMAIN_COM = Domain.of("domain.com");
+    private static final String USERNAME = "user";
+    private static final Username USERMAIL = Username.of(USERNAME + "@" + DOMAIN_COM.name());
+    private static final String PASSWORD = "password";
+    private static final DNSService DNS_SERVICE = null;
 
     @Rule
     public final JUnitSoftAssertions softly = new JUnitSoftAssertions();
@@ -70,18 +77,19 @@ public class JamesMailetContextTest {
     private JamesMailetContext testee;
     private MailAddress mailAddress;
     private MailQueue spoolMailQueue;
+    private MemoryRecipientRewriteTable recipientRewriteTable;
 
     @Before
     @SuppressWarnings("unchecked")
     public void setUp() throws Exception {
-        domainList = new MemoryDomainList(DNS_SERVICE);
+        domainList = spy(new MemoryDomainList(DNS_SERVICE));
         domainList.configure(DomainListConfiguration.builder()
             .autoDetect(false)
             .autoDetectIp(false)
             .build());
 
-        usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
-        MemoryRecipientRewriteTable recipientRewriteTable = new MemoryRecipientRewriteTable();
+        usersRepository = spy(MemoryUsersRepository.withVirtualHosting(domainList));
+        recipientRewriteTable = spy(new MemoryRecipientRewriteTable());
         recipientRewriteTable.configure(new BaseHierarchicalConfiguration());
         MailQueueFactory<MailQueue> mailQueueFactory = mock(MailQueueFactory.class);
         spoolMailQueue = mock(MailQueue.class);
@@ -104,6 +112,52 @@ public class JamesMailetContextTest {
     }
 
     @Test
+    public void isLocalServerShouldPropagateDomainExceptions() throws Exception {
+        when(domainList.getDomains()).thenThrow(new DomainListException("fail!"));
+
+        assertThatThrownBy(() -> testee.isLocalServer(DOMAIN_COM))
+            .isInstanceOf(RuntimeException.class);
+    }
+
+    @Test
+    public void isLocalUserShouldPropagateDomainExceptions() throws Exception {
+        when(domainList.getDefaultDomain()).thenThrow(new DomainListException("fail!"));
+
+        assertThatThrownBy(() -> testee.isLocalUser("user"))
+            .isInstanceOf(RuntimeException.class);
+    }
+
+    @Test
+    public void isLocalUserShouldPropagateUserExceptions() throws Exception {
+        domainList.configure(DomainListConfiguration.builder()
+            .autoDetect(false)
+            .autoDetectIp(false)
+            .defaultDomain(Domain.of("any"))
+            .build());
+        domainList.addDomain(DOMAIN_COM);
+
+        doThrow(new UsersRepositoryException("fail!")).when(usersRepository).contains(any());
+
+        assertThatThrownBy(() -> testee.isLocalUser(USERNAME))
+            .isInstanceOf(RuntimeException.class);
+    }
+
+    @Test
+    public void isLocalUserShouldPropagateRrtExceptions() throws Exception {
+        domainList.configure(DomainListConfiguration.builder()
+            .autoDetect(false)
+            .autoDetectIp(false)
+            .defaultDomain(Domain.of("any"))
+            .build());
+        domainList.addDomain(DOMAIN_COM);
+
+        doThrow(new RecipientRewriteTableException("fail!")).when(recipientRewriteTable).getResolvedMappings(any(), any(), any());
+
+        assertThatThrownBy(() -> testee.isLocalUser(USERNAME))
+            .isInstanceOf(RuntimeException.class);
+    }
+
+    @Test
     public void isLocalServerShouldBeTrueWhenDomainExist() throws Exception {
         domainList.addDomain(DOMAIN_COM);
 
@@ -146,12 +200,12 @@ public class JamesMailetContextTest {
     }
 
     @Test
-    public void isLocalUserShouldBeFalseWhenUsernameDoNotExist() throws Exception {
+    public void isLocalUserShouldBeFalseWhenUsernameDoNotExist() {
         assertThat(testee.isLocalUser(USERMAIL.asString())).isFalse();
     }
 
     @Test
-    public void isLocalEmailShouldBeFalseWhenUsernameDoNotExist() throws Exception {
+    public void isLocalEmailShouldBeFalseWhenUsernameDoNotExist() {
         assertThat(testee.isLocalEmail(mailAddress)).isFalse();
     }
 
@@ -171,11 +225,49 @@ public class JamesMailetContextTest {
     }
 
     @Test
-    public void isLocalEmailShouldBeFalseWhenMailIsNull() throws Exception {
+    public void isLocalEmailShouldBeFalseWhenMailIsNull() {
         assertThat(testee.isLocalEmail(null)).isFalse();
     }
 
     @Test
+    public void isLocalEmailShouldPropagateDomainExceptions() throws Exception {
+        when(domainList.getDomains()).thenThrow(new DomainListException("fail!"));
+
+        assertThatThrownBy(() -> testee.isLocalEmail(mailAddress))
+            .isInstanceOf(RuntimeException.class);
+    }
+
+    @Test
+    public void isLocalEmailShouldPropagateUserExceptions() throws Exception {
+        domainList.configure(DomainListConfiguration.builder()
+            .autoDetect(false)
+            .autoDetectIp(false)
+            .defaultDomain(Domain.of("any"))
+            .build());
+        domainList.addDomain(DOMAIN_COM);
+
+        doThrow(new UsersRepositoryException("fail!")).when(usersRepository).contains(any());
+
+        assertThatThrownBy(() -> testee.isLocalEmail(mailAddress))
+            .isInstanceOf(RuntimeException.class);
+    }
+
+    @Test
+    public void isLocalEmailShouldPropagateRrtExceptions() throws Exception {
+        domainList.configure(DomainListConfiguration.builder()
+            .autoDetect(false)
+            .autoDetectIp(false)
+            .defaultDomain(Domain.of("any"))
+            .build());
+        domainList.addDomain(DOMAIN_COM);
+
+        doThrow(new RecipientRewriteTableException("fail!")).when(recipientRewriteTable).getResolvedMappings(any(), any(), any());
+
+        assertThatThrownBy(() -> testee.isLocalEmail(mailAddress))
+            .isInstanceOf(RuntimeException.class);
+    }
+
+    @Test
     public void bounceShouldNotFailWhenNonConfiguredPostmaster() throws Exception {
         MailImpl mail = MailImpl.builder()
             .name("mail1")


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org