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 f6ec49ad12ee3aa805c2a8371bc449ab82805ba0
Author: Rene Cordier <rcord...@linagora.com>
AuthorDate: Mon Apr 6 11:39:37 2020 +0700
JAMES-3092 Implement VersionParser to parse the version header against an
injected set of supported versions
---
.../org/apache/james/jmap/draft/JMAPModule.java | 4 +++
.../org/apache/james/jmap/JMAPRoutesHandler.java | 38 ++--------------------
.../java/org/apache/james/jmap/JMAPServer.java | 6 ++--
.../main/java/org/apache/james/jmap/Version.java | 18 ----------
.../{JMAPRoutesHandler.java => VersionParser.java} | 38 +++++++++++-----------
.../java/org/apache/james/jmap/JMAPServerTest.java | 22 +++++++++----
.../{VersionTest.java => VersionParserTest.java} | 35 ++++++++++++--------
.../java/org/apache/james/jmap/VersionTest.java | 32 ------------------
8 files changed, 66 insertions(+), 127 deletions(-)
diff --git
a/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/JMAPModule.java
b/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/JMAPModule.java
index b00e73a..850f24e 100644
---
a/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/JMAPModule.java
+++
b/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/JMAPModule.java
@@ -31,6 +31,7 @@ import org.apache.commons.io.FileUtils;
import org.apache.james.filesystem.api.FileSystem;
import org.apache.james.jmap.JMAPConfiguration;
import org.apache.james.jmap.JMAPServer;
+import org.apache.james.jmap.Version;
import org.apache.james.jmap.draft.methods.RequestHandler;
import org.apache.james.jmap.draft.send.PostDequeueDecoratorFactory;
import org.apache.james.jmap.draft.utils.JsoupHtmlTextExtractor;
@@ -105,6 +106,9 @@ public class JMAPModule extends AbstractModule {
bind(MailQueueItemDecoratorFactory.class).to(PostDequeueDecoratorFactory.class).in(Scopes.SINGLETON);
Multibinder.newSetBinder(binder(),
MailboxListener.GroupMailboxListener.class).addBinding().to(PropagateLookupRightListener.class);
+
+ Multibinder<Version> supportedVersions =
Multibinder.newSetBinder(binder(), Version.class);
+ supportedVersions.addBinding().toInstance(Version.DRAFT);
}
@Provides
diff --git
a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java
index 2edb6cc..498b778 100644
---
a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java
+++
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java
@@ -19,22 +19,12 @@
package org.apache.james.jmap;
-import static io.netty.handler.codec.http.HttpHeaderNames.ACCEPT;
-
-import java.util.Arrays;
import java.util.Set;
import java.util.stream.Stream;
-import org.apache.http.NameValuePair;
-import org.apache.http.message.BasicHeaderValueParser;
-
import com.google.common.collect.ImmutableSet;
-import reactor.netty.http.server.HttpServerRequest;
-
public class JMAPRoutesHandler {
- private static String JMAP_VERSION_HEADER = "jmapVersion";
-
private final Version version;
private final Set<JMAPRoutes> routes;
@@ -43,35 +33,11 @@ public class JMAPRoutesHandler {
this.routes = ImmutableSet.copyOf(routes);
}
- Stream<JMAPRoute> routes(HttpServerRequest request) {
- if (matches(request)) {
+ Stream<JMAPRoute> routes(Version versionRequest) {
+ if (version.equals(versionRequest)) {
return routes.stream()
.flatMap(JMAPRoutes::routes);
}
return Stream.of();
}
-
- private boolean matches(HttpServerRequest request) {
- return version.equals(extractRequestVersionHeader(request));
- }
-
- private Version extractRequestVersionHeader(HttpServerRequest request) {
- return asVersion(request)
- .filter(nameValuePair ->
nameValuePair.getName().equals(JMAP_VERSION_HEADER))
- .map(NameValuePair::getValue)
- .map(Version::of)
- .findFirst()
- .orElse(Version.DRAFT);
- }
-
- private Stream<NameValuePair> asVersion(HttpServerRequest request) {
- return request.requestHeaders()
- .getAll(ACCEPT)
- .stream()
- .flatMap(this::extractValueParameters);
- }
-
- private Stream<NameValuePair> extractValueParameters(String value) {
- return Arrays.stream(BasicHeaderValueParser.parseParameters(value,
BasicHeaderValueParser.INSTANCE));
- }
}
diff --git
a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServer.java
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServer.java
index 1bbdd24..6e9b290 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServer.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPServer.java
@@ -41,12 +41,14 @@ public class JMAPServer implements Startable {
private final JMAPConfiguration configuration;
private final Set<JMAPRoutesHandler> jmapRoutesHandlers;
+ private final VersionParser versionParser;
private Optional<DisposableServer> server;
@Inject
- public JMAPServer(JMAPConfiguration configuration, Set<JMAPRoutesHandler>
jmapRoutesHandlers) {
+ public JMAPServer(JMAPConfiguration configuration, Set<JMAPRoutesHandler>
jmapRoutesHandlers, VersionParser versionParser) {
this.configuration = configuration;
this.jmapRoutesHandlers = jmapRoutesHandlers;
+ this.versionParser = versionParser;
this.server = Optional.empty();
}
@@ -75,7 +77,7 @@ public class JMAPServer implements Startable {
private JMAPRoute.Action handleVersionRoute(HttpServerRequest request) {
try {
return jmapRoutesHandlers.stream()
- .flatMap(jmapRoutesHandler ->
jmapRoutesHandler.routes(request))
+ .flatMap(jmapRoutesHandler ->
jmapRoutesHandler.routes(versionParser.parseRequestVersionHeader(request)))
.filter(jmapRoute -> jmapRoute.matches(request))
.map(JMAPRoute::getAction)
.findFirst()
diff --git
a/server/protocols/jmap/src/main/java/org/apache/james/jmap/Version.java
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/Version.java
index 61fd102..7fe2ef5 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/Version.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/Version.java
@@ -19,30 +19,12 @@
package org.apache.james.jmap;
-import java.util.List;
import java.util.Objects;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-
public class Version {
public static final Version DRAFT = new Version("draft");
public static final Version RFC8621 = new Version("rfc-8621");
- private static final List<Version> AVAILABLE_VERSIONS = ImmutableList.of(
- DRAFT,
- RFC8621
- );
-
- public static Version of(String version) {
- Preconditions.checkNotNull(version);
-
- return AVAILABLE_VERSIONS.stream()
- .filter(jmapVersion ->
jmapVersion.version.equalsIgnoreCase(version))
- .findFirst()
- .orElseThrow(() -> new IllegalArgumentException(version + " is not
a supported version"));
- }
-
private final String version;
Version(String version) {
diff --git
a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/VersionParser.java
similarity index 71%
copy from
server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java
copy to
server/protocols/jmap/src/main/java/org/apache/james/jmap/VersionParser.java
index 2edb6cc..0368679 100644
---
a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java
+++
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/VersionParser.java
@@ -25,41 +25,41 @@ import java.util.Arrays;
import java.util.Set;
import java.util.stream.Stream;
+import javax.inject.Inject;
+
import org.apache.http.NameValuePair;
import org.apache.http.message.BasicHeaderValueParser;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
import reactor.netty.http.server.HttpServerRequest;
-public class JMAPRoutesHandler {
- private static String JMAP_VERSION_HEADER = "jmapVersion";
+public class VersionParser {
+ private static final String JMAP_VERSION_HEADER = "jmapVersion";
- private final Version version;
- private final Set<JMAPRoutes> routes;
+ private final Set<Version> supportedVersions;
- public JMAPRoutesHandler(Version version, JMAPRoutes... routes) {
- this.version = version;
- this.routes = ImmutableSet.copyOf(routes);
+ @Inject
+ public VersionParser(Set<Version> supportedVersions) {
+ this.supportedVersions = supportedVersions;
}
- Stream<JMAPRoute> routes(HttpServerRequest request) {
- if (matches(request)) {
- return routes.stream()
- .flatMap(JMAPRoutes::routes);
- }
- return Stream.of();
- }
+ @VisibleForTesting
+ Version parse(String version) {
+ Preconditions.checkNotNull(version);
- private boolean matches(HttpServerRequest request) {
- return version.equals(extractRequestVersionHeader(request));
+ return supportedVersions.stream()
+ .filter(jmapVersion ->
jmapVersion.asString().equalsIgnoreCase(version))
+ .findFirst()
+ .orElseThrow(() -> new IllegalArgumentException(version + " is not
a supported version"));
}
- private Version extractRequestVersionHeader(HttpServerRequest request) {
+ Version parseRequestVersionHeader(HttpServerRequest request) {
return asVersion(request)
.filter(nameValuePair ->
nameValuePair.getName().equals(JMAP_VERSION_HEADER))
.map(NameValuePair::getValue)
- .map(Version::of)
+ .map(this::parse)
.findFirst()
.orElse(Version.DRAFT);
}
diff --git
a/server/protocols/jmap/src/test/java/org/apache/james/jmap/JMAPServerTest.java
b/server/protocols/jmap/src/test/java/org/apache/james/jmap/JMAPServerTest.java
index 759bbb3..992e622 100644
---
a/server/protocols/jmap/src/test/java/org/apache/james/jmap/JMAPServerTest.java
+++
b/server/protocols/jmap/src/test/java/org/apache/james/jmap/JMAPServerTest.java
@@ -81,10 +81,15 @@ class JMAPServerTest {
new FakeJMAPRoutes(AUTHENTICATION_ENDPOINTS, Version.RFC8621))
);
+ private static final ImmutableSet<Version> SUPPORTED_VERSIONS =
ImmutableSet.of(
+ Version.DRAFT,
+ Version.RFC8621
+ );
@Test
void serverShouldAnswerWhenStarted() {
- JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION,
NO_ROUTES_HANDLERS);
+ VersionParser versionParser = new VersionParser(SUPPORTED_VERSIONS);
+ JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION,
NO_ROUTES_HANDLERS, versionParser);
jmapServer.start();
try {
@@ -102,14 +107,16 @@ class JMAPServerTest {
@Test
void startShouldNotThrowWhenConfigurationDisabled() {
- JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION,
NO_ROUTES_HANDLERS);
+ VersionParser versionParser = new VersionParser(SUPPORTED_VERSIONS);
+ JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION,
NO_ROUTES_HANDLERS, versionParser);
assertThatCode(jmapServer::start).doesNotThrowAnyException();
}
@Test
void stopShouldNotThrowWhenConfigurationDisabled() {
- JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION,
NO_ROUTES_HANDLERS);
+ VersionParser versionParser = new VersionParser(SUPPORTED_VERSIONS);
+ JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION,
NO_ROUTES_HANDLERS, versionParser);
jmapServer.start();
assertThatCode(jmapServer::stop).doesNotThrowAnyException();
@@ -117,7 +124,8 @@ class JMAPServerTest {
@Test
void getPortShouldThrowWhenServerIsNotStarted() {
- JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION,
NO_ROUTES_HANDLERS);
+ VersionParser versionParser = new VersionParser(SUPPORTED_VERSIONS);
+ JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION,
NO_ROUTES_HANDLERS, versionParser);
assertThatThrownBy(jmapServer::getPort)
.isInstanceOf(IllegalStateException.class);
@@ -125,7 +133,8 @@ class JMAPServerTest {
@Test
void getPortShouldThrowWhenDisabledConfiguration() {
- JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION,
NO_ROUTES_HANDLERS);
+ VersionParser versionParser = new VersionParser(SUPPORTED_VERSIONS);
+ JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION,
NO_ROUTES_HANDLERS, versionParser);
jmapServer.start();
assertThatThrownBy(jmapServer::getPort)
@@ -138,7 +147,8 @@ class JMAPServerTest {
@BeforeEach
void setUp() {
- server = new JMAPServer(TEST_CONFIGURATION, FAKE_ROUTES_HANDLERS);
+ VersionParser versionParser = new
VersionParser(SUPPORTED_VERSIONS);
+ server = new JMAPServer(TEST_CONFIGURATION, FAKE_ROUTES_HANDLERS,
versionParser);
server.start();
RestAssured.requestSpecification = new RequestSpecBuilder()
diff --git
a/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionTest.java
b/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionParserTest.java
similarity index 67%
copy from
server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionTest.java
copy to
server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionParserTest.java
index c94a8f7..17e1882 100644
--- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionTest.java
+++
b/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionParserTest.java
@@ -22,43 +22,50 @@ package org.apache.james.jmap;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import nl.jqno.equalsverifier.EqualsVerifier;
+import com.google.common.collect.ImmutableSet;
-class VersionTest {
- @Test
- void shouldRespectBeanContract() {
- EqualsVerifier.forClass(Version.class)
- .verify();
+class VersionParserTest {
+ private static final ImmutableSet<Version> SUPPORTED_VERSIONS =
ImmutableSet.of(
+ Version.DRAFT,
+ Version.RFC8621
+ );
+
+ private VersionParser versionParser;
+
+ @BeforeEach
+ void setUp() {
+ versionParser = new VersionParser(SUPPORTED_VERSIONS);
}
@Test
- void ofShouldReturnCorrectValue() {
+ void parseShouldReturnCorrectValue() {
String version = "rfc-8621";
- assertThat(Version.of(version)).isEqualTo(Version.RFC8621);
+ assertThat(versionParser.parse(version)).isEqualTo(Version.RFC8621);
}
@Test
- void ofShouldThrowWhenVersionNotKnown() {
+ void parseShouldThrowWhenVersionNotKnown() {
String version = "unknown";
- assertThatThrownBy(() -> Version.of(version))
+ assertThatThrownBy(() -> versionParser.parse(version))
.isInstanceOf(IllegalArgumentException.class);
}
@Test
- void ofShouldThrowWhenVersionIsNull() {
- assertThatThrownBy(() -> Version.of(null))
+ void parseShouldThrowWhenVersionIsNull() {
+ assertThatThrownBy(() -> versionParser.parse(null))
.isInstanceOf(NullPointerException.class);
}
@Test
- void ofShouldThrowWhenVersionIsEmpty() {
+ void parseShouldThrowWhenVersionIsEmpty() {
String version = "";
- assertThatThrownBy(() -> Version.of(version))
+ assertThatThrownBy(() -> versionParser.parse(version))
.isInstanceOf(IllegalArgumentException.class);
}
}
diff --git
a/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionTest.java
b/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionTest.java
index c94a8f7..704bec3 100644
--- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionTest.java
+++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/VersionTest.java
@@ -19,9 +19,6 @@
package org.apache.james.jmap;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
-
import org.junit.jupiter.api.Test;
import nl.jqno.equalsverifier.EqualsVerifier;
@@ -32,33 +29,4 @@ class VersionTest {
EqualsVerifier.forClass(Version.class)
.verify();
}
-
- @Test
- void ofShouldReturnCorrectValue() {
- String version = "rfc-8621";
-
- assertThat(Version.of(version)).isEqualTo(Version.RFC8621);
- }
-
- @Test
- void ofShouldThrowWhenVersionNotKnown() {
- String version = "unknown";
-
- assertThatThrownBy(() -> Version.of(version))
- .isInstanceOf(IllegalArgumentException.class);
- }
-
- @Test
- void ofShouldThrowWhenVersionIsNull() {
- assertThatThrownBy(() -> Version.of(null))
- .isInstanceOf(NullPointerException.class);
- }
-
- @Test
- void ofShouldThrowWhenVersionIsEmpty() {
- String version = "";
-
- assertThatThrownBy(() -> Version.of(version))
- .isInstanceOf(IllegalArgumentException.class);
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
