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 2019/05/22 03:37:06 UTC

[james-project] 02/03: JAMES-2763 JMAP mailbox capabilities should rely on StartUpCHeck

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 7097478cb26fa5b2b77423b29b1a0ba12b5f869f
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu May 16 10:48:21 2019 +0700

    JAMES-2763 JMAP mailbox capabilities should rely on StartUpCHeck
---
 .../apache/james/JamesCapabilitiesServerTest.java  |  45 +++++++--
 server/container/guice/protocols/jmap/pom.xml      |   5 +
 .../java/org/apache/james/jmap/JMAPModule.java     |  73 +++++++++-----
 .../java/org/apache/james/jmap/JMAPModuleTest.java | 110 +++++++++++++++++++++
 4 files changed, 201 insertions(+), 32 deletions(-)

diff --git a/server/container/guice/cassandra-guice/src/test/java/org/apache/james/JamesCapabilitiesServerTest.java b/server/container/guice/cassandra-guice/src/test/java/org/apache/james/JamesCapabilitiesServerTest.java
index dd28af5..9cca3e8 100644
--- a/server/container/guice/cassandra-guice/src/test/java/org/apache/james/JamesCapabilitiesServerTest.java
+++ b/server/container/guice/cassandra-guice/src/test/java/org/apache/james/JamesCapabilitiesServerTest.java
@@ -19,12 +19,15 @@
 package org.apache.james;
 
 import static org.apache.james.CassandraJamesServerMain.ALL_BUT_JMX_CASSANDRA_MODULE;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.EnumSet;
 
+import org.apache.james.jmap.JMAPModule;
 import org.apache.james.mailbox.MailboxManager;
 import org.apache.james.mailbox.extractor.TextExtractor;
 import org.apache.james.mailbox.store.search.PDFTextExtractor;
@@ -58,7 +61,11 @@ class JamesCapabilitiesServerTest {
         when(mailboxManager.getSupportedSearchCapabilities())
             .thenReturn(EnumSet.allOf(MailboxManager.SearchCapabilities.class));
         
-        assertThatThrownBy(server::start).isInstanceOf(IllegalArgumentException.class);
+        assertThatThrownBy(server::start)
+            .isInstanceOfSatisfying(
+                StartUpChecksPerformer.StartUpChecksException.class,
+                exception -> assertThat(exception.badCheckNames())
+                    .containsOnly(JMAPModule.RequiredCapabilitiesStartUpCheck.CHECK_NAME));
     }
     
     @Test
@@ -70,7 +77,11 @@ class JamesCapabilitiesServerTest {
         when(mailboxManager.getSupportedSearchCapabilities())
             .thenReturn(EnumSet.allOf(MailboxManager.SearchCapabilities.class));
         
-        assertThatThrownBy(server::start).isInstanceOf(IllegalArgumentException.class);
+        assertThatThrownBy(server::start)
+            .isInstanceOfSatisfying(
+                StartUpChecksPerformer.StartUpChecksException.class,
+                exception -> assertThat(exception.badCheckNames())
+                    .containsOnly(JMAPModule.RequiredCapabilitiesStartUpCheck.CHECK_NAME));
     }
     
     @Test
@@ -82,7 +93,11 @@ class JamesCapabilitiesServerTest {
         when(mailboxManager.getSupportedSearchCapabilities())
             .thenReturn(EnumSet.complementOf(EnumSet.of(MailboxManager.SearchCapabilities.Attachment)));
 
-        assertThatThrownBy(server::start).isInstanceOf(IllegalArgumentException.class);
+        assertThatThrownBy(server::start)
+            .isInstanceOfSatisfying(
+                StartUpChecksPerformer.StartUpChecksException.class,
+                exception -> assertThat(exception.badCheckNames())
+                    .containsOnly(JMAPModule.RequiredCapabilitiesStartUpCheck.CHECK_NAME));
     }
 
     @Test
@@ -94,7 +109,11 @@ class JamesCapabilitiesServerTest {
         when(mailboxManager.getSupportedSearchCapabilities())
             .thenReturn(EnumSet.complementOf(EnumSet.of(MailboxManager.SearchCapabilities.AttachmentFileName)));
 
-        assertThatThrownBy(server::start).isInstanceOf(IllegalArgumentException.class);
+        assertThatThrownBy(server::start)
+            .isInstanceOfSatisfying(
+                StartUpChecksPerformer.StartUpChecksException.class,
+                exception -> assertThat(exception.badCheckNames())
+                    .containsOnly(JMAPModule.RequiredCapabilitiesStartUpCheck.CHECK_NAME));
     }
     
     @Test
@@ -106,7 +125,11 @@ class JamesCapabilitiesServerTest {
         when(mailboxManager.getSupportedSearchCapabilities())
             .thenReturn(EnumSet.complementOf(EnumSet.of(MailboxManager.SearchCapabilities.MultimailboxSearch)));
 
-        assertThatThrownBy(server::start).isInstanceOf(IllegalArgumentException.class);
+        assertThatThrownBy(server::start)
+            .isInstanceOfSatisfying(
+                StartUpChecksPerformer.StartUpChecksException.class,
+                exception -> assertThat(exception.badCheckNames())
+                    .containsOnly(JMAPModule.RequiredCapabilitiesStartUpCheck.CHECK_NAME));
     }
 
     @Test
@@ -118,7 +141,11 @@ class JamesCapabilitiesServerTest {
         when(mailboxManager.getSupportedSearchCapabilities())
             .thenReturn(EnumSet.allOf(MailboxManager.SearchCapabilities.class));
 
-        assertThatThrownBy(server::start).isInstanceOf(IllegalArgumentException.class);
+        assertThatThrownBy(server::start)
+            .isInstanceOfSatisfying(
+                StartUpChecksPerformer.StartUpChecksException.class,
+                exception -> assertThat(exception.badCheckNames())
+                    .containsOnly(JMAPModule.RequiredCapabilitiesStartUpCheck.CHECK_NAME));
     }
 
     @Test
@@ -130,7 +157,11 @@ class JamesCapabilitiesServerTest {
         when(mailboxManager.getSupportedSearchCapabilities())
             .thenReturn(EnumSet.allOf(MailboxManager.SearchCapabilities.class));
 
-        server.start();
+        assertThatCode(server::start)
+            .doesNotThrowAnyException();
+
+        assertThat(server.isStarted())
+            .isTrue();
     }
 
 }
diff --git a/server/container/guice/protocols/jmap/pom.xml b/server/container/guice/protocols/jmap/pom.xml
index 2beb647..eff914c 100644
--- a/server/container/guice/protocols/jmap/pom.xml
+++ b/server/container/guice/protocols/jmap/pom.xml
@@ -94,6 +94,11 @@
             <artifactId>junit-vintage-engine</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>
diff --git a/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPModule.java b/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPModule.java
index 044759d..02a24b0 100644
--- a/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPModule.java
+++ b/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPModule.java
@@ -22,8 +22,8 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.EnumSet;
-import java.util.List;
 import java.util.Optional;
+import java.util.stream.Stream;
 
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
@@ -37,7 +37,7 @@ import org.apache.james.jmap.send.PostDequeueDecoratorFactory;
 import org.apache.james.jmap.utils.HtmlTextExtractor;
 import org.apache.james.jmap.utils.JsoupHtmlTextExtractor;
 import org.apache.james.jwt.JwtConfiguration;
-import org.apache.james.lifecycle.api.Startable;
+import org.apache.james.lifecycle.api.StartUpCheck;
 import org.apache.james.mailbox.MailboxManager;
 import org.apache.james.mailbox.MailboxManager.SearchCapabilities;
 import org.apache.james.mailbox.events.MailboxListener;
@@ -45,13 +45,13 @@ import org.apache.james.modules.server.CamelMailetContainerModule;
 import org.apache.james.queue.api.MailQueueItemDecoratorFactory;
 import org.apache.james.server.core.configuration.FileConfigurationProvider;
 import org.apache.james.transport.matchers.RecipientIsLocal;
-import org.apache.james.utils.ConfigurationPerformer;
 import org.apache.james.utils.PropertiesProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.github.fge.lambdas.Throwing;
-import com.google.common.base.Preconditions;
+import com.github.steveash.guavate.Guavate;
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
@@ -94,7 +94,7 @@ public class JMAPModule extends AbstractModule {
         bind(JsoupHtmlTextExtractor.class).in(Scopes.SINGLETON);
 
         bind(HtmlTextExtractor.class).to(JsoupHtmlTextExtractor.class);
-        Multibinder.newSetBinder(binder(), ConfigurationPerformer.class).addBinding().to(RequiredCapabilitiesPrecondition.class);
+        Multibinder.newSetBinder(binder(), StartUpCheck.class).addBinding().to(RequiredCapabilitiesStartUpCheck.class);
 
         Multibinder<CamelMailetContainerModule.TransportProcessorCheck> transportProcessorChecks = Multibinder.newSetBinder(binder(), CamelMailetContainerModule.TransportProcessorCheck.class);
         transportProcessorChecks.addBinding().toInstance(VACATION_MAILET_CHECK);
@@ -136,39 +136,62 @@ public class JMAPModule extends AbstractModule {
     }
 
     @Singleton
-    public static class RequiredCapabilitiesPrecondition implements ConfigurationPerformer {
+    public static class RequiredCapabilitiesStartUpCheck implements StartUpCheck {
+
+        public static final String CHECK_NAME = "MailboxCapabilitiesForJMAP";
 
         private final MailboxManager mailboxManager;
 
         @Inject
-        public RequiredCapabilitiesPrecondition(MailboxManager mailboxManager) {
+        public RequiredCapabilitiesStartUpCheck(MailboxManager mailboxManager) {
             this.mailboxManager = mailboxManager;
         }
 
         @Override
-        public void initModule() {
-            Preconditions.checkArgument(mailboxManager.hasCapability(MailboxManager.MailboxCapabilities.Move),
-                    "MOVE support in MailboxManager is required by JMAP Module");
-            Preconditions.checkArgument(mailboxManager.hasCapability(MailboxManager.MailboxCapabilities.ACL),
-                    "ACL support in MailboxManager is required by JMAP Module");
-
+        public CheckResult check() {
             EnumSet<MailboxManager.MessageCapabilities> messageCapabilities = mailboxManager.getSupportedMessageCapabilities();
-            Preconditions.checkArgument(messageCapabilities.contains(MailboxManager.MessageCapabilities.UniqueID),
-                    "MessageIdManager is not defined by this Mailbox implementation");
-
             EnumSet<SearchCapabilities> searchCapabilities = mailboxManager.getSupportedSearchCapabilities();
-            Preconditions.checkArgument(searchCapabilities.contains(MailboxManager.SearchCapabilities.MultimailboxSearch),
-                    "Multimailbox search in MailboxManager is required by JMAP Module");
-            Preconditions.checkArgument(searchCapabilities.contains(MailboxManager.SearchCapabilities.Attachment),
-                    "Attachment Search support in MailboxManager is required by JMAP Module");
-            Preconditions.checkArgument(searchCapabilities.contains(SearchCapabilities.AttachmentFileName),
-                    "Attachment file name Search support in MailboxManager is required by JMAP Module");
+
+            ImmutableList<String> badCheckDescriptions = Stream.of(
+                    badCheckDescription(mailboxManager.hasCapability(MailboxManager.MailboxCapabilities.Move),
+                        "MOVE support in MailboxManager is required by JMAP Module"),
+                    badCheckDescription(mailboxManager.hasCapability(MailboxManager.MailboxCapabilities.ACL),
+                        "ACL support in MailboxManager is required by JMAP Module"),
+                    badCheckDescription(messageCapabilities.contains(MailboxManager.MessageCapabilities.UniqueID),
+                        "MessageIdManager is not defined by this Mailbox implementation"),
+                    badCheckDescription(searchCapabilities.contains(SearchCapabilities.MultimailboxSearch),
+                        "Multimailbox search in MailboxManager is required by JMAP Module"),
+                    badCheckDescription(searchCapabilities.contains(SearchCapabilities.Attachment),
+                        "Attachment Search support in MailboxManager is required by JMAP Module"),
+                    badCheckDescription(searchCapabilities.contains(SearchCapabilities.AttachmentFileName),
+                    "Attachment file name Search support in MailboxManager is required by JMAP Module"))
+                .filter(Optional::isPresent)
+                .map(Optional::get)
+                .collect(Guavate.toImmutableList());
+
+            if (!badCheckDescriptions.isEmpty()) {
+                return CheckResult.builder()
+                    .checkName(checkName())
+                    .resultType(ResultType.BAD)
+                    .description(Joiner.on(",").join(badCheckDescriptions))
+                    .build();
+            }
+            return CheckResult.builder()
+                .checkName(checkName())
+                .resultType(ResultType.GOOD)
+                .build();
+        }
+
+        private Optional<String> badCheckDescription(boolean expressionResult, String expressionFailsDescription) {
+            if (expressionResult) {
+                return Optional.empty();
+            }
+            return Optional.ofNullable(expressionFailsDescription);
         }
 
         @Override
-        public List<Class<? extends Startable>> forClasses() {
-            return ImmutableList.of();
+        public String checkName() {
+            return CHECK_NAME;
         }
     }
-
 }
diff --git a/server/container/guice/protocols/jmap/src/test/java/org/apache/james/jmap/JMAPModuleTest.java b/server/container/guice/protocols/jmap/src/test/java/org/apache/james/jmap/JMAPModuleTest.java
new file mode 100644
index 0000000..c4f2f6d
--- /dev/null
+++ b/server/container/guice/protocols/jmap/src/test/java/org/apache/james/jmap/JMAPModuleTest.java
@@ -0,0 +1,110 @@
+/****************************************************************
+ * 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.jmap;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.EnumSet;
+
+import org.apache.james.jmap.JMAPModule.RequiredCapabilitiesStartUpCheck;
+import org.apache.james.mailbox.MailboxManager;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+class JMAPModuleTest {
+
+    @Nested
+    class RequiredCapabilitiesStartUpCheckTest {
+
+        private RequiredCapabilitiesStartUpCheck testee;
+        private MailboxManager mockMailboxManager;
+        private EnumSet<MailboxManager.MessageCapabilities> mockMessageCapabilities;
+        private EnumSet<MailboxManager.SearchCapabilities> mockSearchCapabilities;
+
+        @BeforeEach
+        void beforeEach() {
+            mockMailboxManager = mock(MailboxManager.class);
+            mockMessageCapabilities = (EnumSet<MailboxManager.MessageCapabilities>) mock(EnumSet.class);
+            mockSearchCapabilities = (EnumSet<MailboxManager.SearchCapabilities>) mock(EnumSet.class);
+            when(mockMailboxManager.getSupportedMessageCapabilities())
+                .thenReturn(mockMessageCapabilities);
+            when(mockMailboxManager.getSupportedSearchCapabilities())
+                .thenReturn(mockSearchCapabilities);
+
+            testee = new RequiredCapabilitiesStartUpCheck(mockMailboxManager);
+        }
+
+        @Test
+        void checkShouldReturnGoodWhenAllChecksSatisfy() {
+            when(mockMailboxManager.hasCapability(any()))
+                .thenReturn(true);
+            when(mockMessageCapabilities.contains(any()))
+                .thenReturn(true);
+            when(mockSearchCapabilities.contains(any()))
+                .thenReturn(true);
+
+            assertThat(testee.check().isGood())
+                .isTrue();
+        }
+
+        @Test
+        void checkShouldReturnBadWhenMailboxManagerDoesntHaveCapabilities() {
+            when(mockMailboxManager.hasCapability(any()))
+                .thenReturn(false);
+            when(mockMessageCapabilities.contains(any()))
+                .thenReturn(true);
+            when(mockSearchCapabilities.contains(any()))
+                .thenReturn(true);
+
+            assertThat(testee.check().isBad())
+                .isTrue();
+        }
+
+        @Test
+        void checkShouldReturnBadWhenMailboxManagerDoesntHaveMessagesCapabilities() {
+            when(mockMailboxManager.hasCapability(any()))
+                .thenReturn(true);
+            when(mockMessageCapabilities.contains(any()))
+                .thenReturn(false);
+            when(mockSearchCapabilities.contains(any()))
+                .thenReturn(true);
+
+            assertThat(testee.check().isBad())
+                .isTrue();
+        }
+
+        @Test
+        void checkShouldReturnBadWhenMailboxManagerDoesntHaveSearchCapabilities() {
+            when(mockMailboxManager.hasCapability(any()))
+                .thenReturn(true);
+            when(mockMessageCapabilities.contains(any()))
+                .thenReturn(true);
+            when(mockSearchCapabilities.contains(any()))
+                .thenReturn(false);
+
+            assertThat(testee.check().isBad())
+                .isTrue();
+        }
+    }
+}
\ No newline at end of file


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