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 075a6291f08b4c261cd3ef7261dbb668ae5d24d9 Author: Rene Cordier <rcord...@linagora.com> AuthorDate: Mon Mar 30 15:54:26 2020 +0700 JAMES-3092 Add a JMAPRoutesHandler component to handle version and routes injection --- .../modules/protocols/JMAPDraftServerModule.java | 21 ++++++---- .../james/jmap/http/AuthenticationRoutes.java | 9 ++--- .../org/apache/james/jmap/http/DownloadRoutes.java | 13 +++--- .../org/apache/james/jmap/http/JMAPApiRoutes.java | 5 +-- .../org/apache/james/jmap/http/UploadRoutes.java | 5 +-- .../main/java/org/apache/james/jmap/JMAPRoute.java | 32 +-------------- .../{JMAPRoute.java => JMAPRoutesHandler.java} | 47 +++++++++------------- .../java/org/apache/james/jmap/JMAPServer.java | 12 +++--- .../java/org/apache/james/jmap/JMAPServerTest.java | 30 ++++++++------ 9 files changed, 71 insertions(+), 103 deletions(-) diff --git a/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/modules/protocols/JMAPDraftServerModule.java b/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/modules/protocols/JMAPDraftServerModule.java index 97ee0c2..2ab7049 100644 --- a/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/modules/protocols/JMAPDraftServerModule.java +++ b/server/container/guice/protocols/jmap-draft/src/main/java/org/apache/james/modules/protocols/JMAPDraftServerModule.java @@ -22,8 +22,9 @@ package org.apache.james.modules.protocols; import java.security.Security; import org.apache.james.jmap.JMAPConfiguration; -import org.apache.james.jmap.JMAPRoutes; +import org.apache.james.jmap.JMAPRoutesHandler; import org.apache.james.jmap.JMAPServer; +import org.apache.james.jmap.Version; import org.apache.james.jmap.draft.JMAPModule; import org.apache.james.jmap.draft.JmapGuiceProbe; import org.apache.james.jmap.draft.MessageIdProbe; @@ -48,12 +49,6 @@ public class JMAPDraftServerModule extends AbstractModule { install(new JMAPModule()); Multibinder.newSetBinder(binder(), GuiceProbe.class).addBinding().to(JmapGuiceProbe.class); Multibinder.newSetBinder(binder(), GuiceProbe.class).addBinding().to(MessageIdProbe.class); - Multibinder<JMAPRoutes> routesBinder = Multibinder.newSetBinder(binder(), JMAPRoutes.class); - - routesBinder.addBinding().to(AuthenticationRoutes.class); - routesBinder.addBinding().to(JMAPApiRoutes.class); - routesBinder.addBinding().to(UploadRoutes.class); - routesBinder.addBinding().to(DownloadRoutes.class); } @ProvidesIntoSet @@ -69,6 +64,18 @@ public class JMAPDraftServerModule extends AbstractModule { }); } + @ProvidesIntoSet + JMAPRoutesHandler routesHandler(AuthenticationRoutes authenticationRoutes, + JMAPApiRoutes jmapApiRoutes, + UploadRoutes uploadRoutes, + DownloadRoutes downloadRoutes) { + return new JMAPRoutesHandler(Version.DRAFT, + authenticationRoutes, + jmapApiRoutes, + uploadRoutes, + downloadRoutes); + } + private void registerPEMWithSecurityProvider() { Security.addProvider(new BouncyCastleProvider()); } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java index 9412861..f9f9eb1 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java @@ -46,7 +46,6 @@ import org.apache.james.jmap.Endpoint; import org.apache.james.jmap.JMAPRoute; import org.apache.james.jmap.JMAPRoutes; import org.apache.james.jmap.JMAPUrls; -import org.apache.james.jmap.Version; import org.apache.james.jmap.api.access.AccessToken; import org.apache.james.jmap.draft.api.AccessTokenManager; import org.apache.james.jmap.draft.api.SimpleTokenFactory; @@ -108,10 +107,10 @@ public class AuthenticationRoutes implements JMAPRoutes { @Override public Stream<JMAPRoute> routes() { return Stream.of( - new JMAPRoute(new Endpoint(HttpMethod.POST, AUTHENTICATION), Version.DRAFT, JMAPRoutes.corsHeaders(this::post)), - new JMAPRoute(new Endpoint(HttpMethod.GET, AUTHENTICATION), Version.DRAFT, JMAPRoutes.corsHeaders(this::returnEndPointsResponse)), - new JMAPRoute(new Endpoint(HttpMethod.DELETE, AUTHENTICATION), Version.DRAFT, JMAPRoutes.corsHeaders(this::delete)), - new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, AUTHENTICATION), Version.DRAFT, CORS_CONTROL) + new JMAPRoute(new Endpoint(HttpMethod.POST, AUTHENTICATION), JMAPRoutes.corsHeaders(this::post)), + new JMAPRoute(new Endpoint(HttpMethod.GET, AUTHENTICATION), JMAPRoutes.corsHeaders(this::returnEndPointsResponse)), + new JMAPRoute(new Endpoint(HttpMethod.DELETE, AUTHENTICATION), JMAPRoutes.corsHeaders(this::delete)), + new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, AUTHENTICATION), CORS_CONTROL) ); } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java index c8b35a2..e0cc639 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java @@ -40,7 +40,6 @@ import javax.inject.Inject; import org.apache.james.jmap.Endpoint; import org.apache.james.jmap.JMAPRoute; import org.apache.james.jmap.JMAPRoutes; -import org.apache.james.jmap.Version; import org.apache.james.jmap.draft.api.SimpleTokenFactory; import org.apache.james.jmap.draft.exceptions.BadRequestException; import org.apache.james.jmap.draft.exceptions.InternalErrorException; @@ -101,12 +100,12 @@ public class DownloadRoutes implements JMAPRoutes { @Override public Stream<JMAPRoute> routes() { return Stream.of( - new JMAPRoute(new Endpoint(HttpMethod.POST, DOWNLOAD_FROM_ID), Version.DRAFT, JMAPRoutes.corsHeaders(this::postFromId)), - new JMAPRoute(new Endpoint(HttpMethod.GET, DOWNLOAD_FROM_ID), Version.DRAFT, JMAPRoutes.corsHeaders(this::getFromId)), - new JMAPRoute(new Endpoint(HttpMethod.POST, DOWNLOAD_FROM_ID_AND_NAME), Version.DRAFT, JMAPRoutes.corsHeaders(this::postFromIdAndName)), - new JMAPRoute(new Endpoint(HttpMethod.GET, DOWNLOAD_FROM_ID_AND_NAME), Version.DRAFT, JMAPRoutes.corsHeaders(this::getFromIdAndName)), - new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, DOWNLOAD_FROM_ID), Version.DRAFT, CORS_CONTROL), - new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, DOWNLOAD_FROM_ID_AND_NAME), Version.DRAFT, CORS_CONTROL) + new JMAPRoute(new Endpoint(HttpMethod.POST, DOWNLOAD_FROM_ID), JMAPRoutes.corsHeaders(this::postFromId)), + new JMAPRoute(new Endpoint(HttpMethod.GET, DOWNLOAD_FROM_ID), JMAPRoutes.corsHeaders(this::getFromId)), + new JMAPRoute(new Endpoint(HttpMethod.POST, DOWNLOAD_FROM_ID_AND_NAME), JMAPRoutes.corsHeaders(this::postFromIdAndName)), + new JMAPRoute(new Endpoint(HttpMethod.GET, DOWNLOAD_FROM_ID_AND_NAME), JMAPRoutes.corsHeaders(this::getFromIdAndName)), + new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, DOWNLOAD_FROM_ID), CORS_CONTROL), + new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, DOWNLOAD_FROM_ID_AND_NAME), CORS_CONTROL) ); } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java index f574bff..6530171 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java @@ -34,7 +34,6 @@ import javax.inject.Inject; import org.apache.james.jmap.Endpoint; import org.apache.james.jmap.JMAPRoute; import org.apache.james.jmap.JMAPRoutes; -import org.apache.james.jmap.Version; import org.apache.james.jmap.draft.exceptions.BadRequestException; import org.apache.james.jmap.draft.exceptions.InternalErrorException; import org.apache.james.jmap.draft.exceptions.UnauthorizedException; @@ -88,8 +87,8 @@ public class JMAPApiRoutes implements JMAPRoutes { @Override public Stream<JMAPRoute> routes() { return Stream.of( - new JMAPRoute(new Endpoint(HttpMethod.POST, JMAP), Version.DRAFT, JMAPRoutes.corsHeaders(this::post)), - new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, JMAP), Version.DRAFT, CORS_CONTROL) + new JMAPRoute(new Endpoint(HttpMethod.POST, JMAP), JMAPRoutes.corsHeaders(this::post)), + new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, JMAP), CORS_CONTROL) ); } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java index 92c289d..b0f22ba 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java @@ -38,7 +38,6 @@ import javax.inject.Inject; import org.apache.james.jmap.Endpoint; import org.apache.james.jmap.JMAPRoute; import org.apache.james.jmap.JMAPRoutes; -import org.apache.james.jmap.Version; import org.apache.james.jmap.draft.exceptions.BadRequestException; import org.apache.james.jmap.draft.exceptions.InternalErrorException; import org.apache.james.jmap.draft.exceptions.UnauthorizedException; @@ -90,8 +89,8 @@ public class UploadRoutes implements JMAPRoutes { @Override public Stream<JMAPRoute> routes() { return Stream.of( - new JMAPRoute(new Endpoint(HttpMethod.POST, UPLOAD), Version.DRAFT, JMAPRoutes.corsHeaders(this::post)), - new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, UPLOAD), Version.DRAFT, CORS_CONTROL) + new JMAPRoute(new Endpoint(HttpMethod.POST, UPLOAD), JMAPRoutes.corsHeaders(this::post)), + new JMAPRoute(new Endpoint(HttpMethod.OPTIONS, UPLOAD), CORS_CONTROL) ); } diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoute.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoute.java index aaa3909..c28cc73 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoute.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoute.java @@ -19,12 +19,6 @@ package org.apache.james.jmap; -import static io.netty.handler.codec.http.HttpHeaderNames.ACCEPT; - -import java.util.Arrays; -import java.util.Optional; -import java.util.stream.Stream; - import org.reactivestreams.Publisher; import reactor.netty.http.server.HttpServerRequest; @@ -35,15 +29,11 @@ public class JMAPRoute { Publisher<Void> handleRequest(HttpServerRequest request, HttpServerResponse response); } - private static final String JMAP_VERSION_HEADER = "jmapVersion="; - private final Endpoint endpoint; - private final Version version; private final Action action; - public JMAPRoute(Endpoint endpoint, Version version, Action action) { + public JMAPRoute(Endpoint endpoint, Action action) { this.endpoint = endpoint; - this.version = version; this.action = action; } @@ -51,29 +41,11 @@ public class JMAPRoute { return endpoint; } - public Version getVersion() { - return version; - } - public Action getAction() { return action; } public boolean matches(HttpServerRequest request) { - return getVersion().equals(extractRequestVersionHeader(request)) - && getEndpoint().matches(request); - } - - private Version extractRequestVersionHeader(HttpServerRequest request) { - return Optional.ofNullable(request.requestHeaders().get(ACCEPT)) - .map(s -> s.split(";")) - .map(Arrays::stream) - .orElse(Stream.of()) - .map(value -> value.trim().toLowerCase()) - .filter(value -> value.startsWith(JMAP_VERSION_HEADER.toLowerCase())) - .map(value -> value.substring(JMAP_VERSION_HEADER.length())) - .map(Version::of) - .findFirst() - .orElse(Version.DRAFT); + return endpoint.matches(request); } } diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoute.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java similarity index 66% copy from server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoute.java copy to server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java index aaa3909..8c094f5 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoute.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutesHandler.java @@ -23,52 +23,41 @@ import static io.netty.handler.codec.http.HttpHeaderNames.ACCEPT; import java.util.Arrays; import java.util.Optional; +import java.util.Set; import java.util.stream.Stream; -import org.reactivestreams.Publisher; +import com.google.common.collect.ImmutableSet; import reactor.netty.http.server.HttpServerRequest; -import reactor.netty.http.server.HttpServerResponse; -public class JMAPRoute { - public interface Action { - Publisher<Void> handleRequest(HttpServerRequest request, HttpServerResponse response); - } - - private static final String JMAP_VERSION_HEADER = "jmapVersion="; +public class JMAPRoutesHandler { + String JMAP_VERSION_HEADER = "jmapVersion="; - private final Endpoint endpoint; private final Version version; - private final Action action; + private final Set<JMAPRoutes> routes; - public JMAPRoute(Endpoint endpoint, Version version, Action action) { - this.endpoint = endpoint; + public JMAPRoutesHandler(Version version, JMAPRoutes... routes) { this.version = version; - this.action = action; - } - - public Endpoint getEndpoint() { - return endpoint; - } - - public Version getVersion() { - return version; + this.routes = ImmutableSet.copyOf(routes); } - public Action getAction() { - return action; + Stream<JMAPRoute> routes(HttpServerRequest request) { + if (matches(request)) { + return routes.stream() + .flatMap(JMAPRoutes::routes); + } + return Stream.of(); } - public boolean matches(HttpServerRequest request) { - return getVersion().equals(extractRequestVersionHeader(request)) - && getEndpoint().matches(request); + private boolean matches(HttpServerRequest request) { + return version.equals(extractRequestVersionHeader(request)); } private Version extractRequestVersionHeader(HttpServerRequest request) { return Optional.ofNullable(request.requestHeaders().get(ACCEPT)) - .map(s -> s.split(";")) - .map(Arrays::stream) - .orElse(Stream.of()) + .map(s -> s.split(";")) + .map(Arrays::stream) + .orElse(Stream.of()) .map(value -> value.trim().toLowerCase()) .filter(value -> value.startsWith(JMAP_VERSION_HEADER.toLowerCase())) .map(value -> value.substring(JMAP_VERSION_HEADER.length())) 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 6b354fe..1bbdd24 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 @@ -40,13 +40,13 @@ public class JMAPServer implements Startable { private static final int RANDOM_PORT = 0; private final JMAPConfiguration configuration; - private final Set<JMAPRoutes> jmapRoutes; + private final Set<JMAPRoutesHandler> jmapRoutesHandlers; private Optional<DisposableServer> server; @Inject - public JMAPServer(JMAPConfiguration configuration, Set<JMAPRoutes> jmapRoutes) { + public JMAPServer(JMAPConfiguration configuration, Set<JMAPRoutesHandler> jmapRoutesHandlers) { this.configuration = configuration; - this.jmapRoutes = jmapRoutes; + this.jmapRoutesHandlers = jmapRoutesHandlers; this.server = Optional.empty(); } @@ -74,8 +74,8 @@ public class JMAPServer implements Startable { private JMAPRoute.Action handleVersionRoute(HttpServerRequest request) { try { - return jmapRoutes.stream() - .flatMap(JMAPRoutes::routes) + return jmapRoutesHandlers.stream() + .flatMap(jmapRoutesHandler -> jmapRoutesHandler.routes(request)) .filter(jmapRoute -> jmapRoute.matches(request)) .map(JMAPRoute::getAction) .findFirst() @@ -85,8 +85,6 @@ public class JMAPServer implements Startable { } } - - @PreDestroy public void stop() { server.ifPresent(DisposableServer::disposeNow); 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 a0eec47..fe2e2f9 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 @@ -60,7 +60,7 @@ class JMAPServerTest { .enable() .randomPort() .build(); - private static final ImmutableSet<JMAPRoutes> NO_ROUTES = ImmutableSet.of(); + private static final ImmutableSet<JMAPRoutesHandler> NO_ROUTES_HANDLERS = ImmutableSet.of(); private static final ImmutableSet<Endpoint> AUTHENTICATION_ENDPOINTS = ImmutableSet.of( new Endpoint(HttpMethod.POST, JMAPUrls.AUTHENTICATION), @@ -70,15 +70,21 @@ class JMAPServerTest { new Endpoint(HttpMethod.POST, JMAPUrls.JMAP), new Endpoint(HttpMethod.DELETE, JMAPUrls.JMAP) ); - private static final ImmutableSet<JMAPRoutes> FAKE_ROUTES = ImmutableSet.of( - new FakeJMAPRoutes(AUTHENTICATION_ENDPOINTS, Version.DRAFT), - new FakeJMAPRoutes(AUTHENTICATION_ENDPOINTS, Version.RFC8621), - new FakeJMAPRoutes(JMAP_ENDPOINTS, Version.DRAFT) + + private static final ImmutableSet<JMAPRoutesHandler> FAKE_ROUTES_HANDLERS = ImmutableSet.of( + new JMAPRoutesHandler( + Version.DRAFT, + new FakeJMAPRoutes(AUTHENTICATION_ENDPOINTS, Version.DRAFT), + new FakeJMAPRoutes(JMAP_ENDPOINTS, Version.DRAFT)), + new JMAPRoutesHandler( + Version.RFC8621, + new FakeJMAPRoutes(AUTHENTICATION_ENDPOINTS, Version.RFC8621)) ); + @Test void serverShouldAnswerWhenStarted() { - JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION, NO_ROUTES); + JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION, NO_ROUTES_HANDLERS); jmapServer.start(); try { @@ -96,14 +102,14 @@ class JMAPServerTest { @Test void startShouldNotThrowWhenConfigurationDisabled() { - JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION, NO_ROUTES); + JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION, NO_ROUTES_HANDLERS); assertThatCode(jmapServer::start).doesNotThrowAnyException(); } @Test void stopShouldNotThrowWhenConfigurationDisabled() { - JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION, NO_ROUTES); + JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION, NO_ROUTES_HANDLERS); jmapServer.start(); assertThatCode(jmapServer::stop).doesNotThrowAnyException(); @@ -111,7 +117,7 @@ class JMAPServerTest { @Test void getPortShouldThrowWhenServerIsNotStarted() { - JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION, NO_ROUTES); + JMAPServer jmapServer = new JMAPServer(TEST_CONFIGURATION, NO_ROUTES_HANDLERS); assertThatThrownBy(jmapServer::getPort) .isInstanceOf(IllegalStateException.class); @@ -119,7 +125,7 @@ class JMAPServerTest { @Test void getPortShouldThrowWhenDisabledConfiguration() { - JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION, NO_ROUTES); + JMAPServer jmapServer = new JMAPServer(DISABLED_CONFIGURATION, NO_ROUTES_HANDLERS); jmapServer.start(); assertThatThrownBy(jmapServer::getPort) @@ -132,7 +138,7 @@ class JMAPServerTest { @BeforeEach void setUp() { - server = new JMAPServer(TEST_CONFIGURATION, FAKE_ROUTES); + server = new JMAPServer(TEST_CONFIGURATION, FAKE_ROUTES_HANDLERS); server.start(); RestAssured.requestSpecification = new RequestSpecBuilder() @@ -220,7 +226,7 @@ class JMAPServerTest { @Override public Stream<JMAPRoute> routes() { return endpoints.stream() - .map(endpoint -> new JMAPRoute(endpoint, version, (request, response) -> sendVersionResponse(response))); + .map(endpoint -> new JMAPRoute(endpoint, (request, response) -> sendVersionResponse(response))); } @Override --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org