You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2020/11/04 10:41:33 UTC
[GitHub] [james-project] chibenwa commented on a change in pull request #256: JAMES-3389, add api to webadmin server for receiving mail over rest i…
chibenwa commented on a change in pull request #256:
URL: https://github.com/apache/james-project/pull/256#discussion_r517250434
##########
File path: server/protocols/webadmin/webadmin-mail-over-web/src/test/java/org/apache/james/routes/ReceiveMailOverWebRoutesTest.java
##########
@@ -0,0 +1,91 @@
+package org.apache.james.routes;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static io.restassured.config.EncoderConfig.encoderConfig;
+import static io.restassured.config.RestAssuredConfig.newConfig;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.james.queue.api.MailQueueFactory;
+import org.apache.james.queue.api.RawMailQueueItemDecoratorFactory;
+import org.apache.james.queue.memory.MemoryMailQueueFactory;
+import org.apache.james.util.ClassLoaderUtils;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.routes.ReceiveMailOverWebRoutes;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import io.restassured.RestAssured;
+import io.restassured.builder.RequestSpecBuilder;
+import io.restassured.http.ContentType;
+import io.restassured.specification.RequestSpecification;
+
+public class ReceiveMailOverWebRoutesTest {
Review comment:
We only test the response code here.
Maybe testing the content of the mail queue can be valuable too?
##########
File path: server/container/guice/pom.xml
##########
@@ -189,6 +190,46 @@
<artifactId>james-server-guice-webadmin-jmap</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
+ <artifactId>james-server-guice-webadmin-mail-over-web</artifactId>
+ <version>${project.version}</version>
+ </dependency>
Review comment:
Why adding the dependency 8 times?
##########
File path: server/container/guice/protocols/webadmin-mail-over-web/src/main/java/org/apache/james/modules/server/WebAdminMailOverWebModule.java
##########
@@ -0,0 +1,16 @@
+package org.apache.james.modules.server;
Review comment:
The Apache V2 license header is compulsory.
##########
File path: server/protocols/webadmin/webadmin-mail-over-web/src/main/java/org/apache/james/webadmin/routes/ReceiveMailOverWebRoutes.java
##########
@@ -0,0 +1,80 @@
+package org.apache.james.webadmin.routes;
+
+import java.io.ByteArrayInputStream;
+import java.util.UUID;
+
+import javax.inject.Inject;
+import javax.mail.Session;
+import javax.mail.internet.MimeMessage;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+
+import org.apache.james.queue.api.MailQueue;
+import org.apache.james.queue.api.MailQueueFactory;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractor;
+import org.eclipse.jetty.http.HttpStatus;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiImplicitParam;
+import io.swagger.annotations.ApiImplicitParams;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import spark.Service;
+
+@Api(tags = "OverWebMailReceiver")
+@Path(ReceiveMailOverWebRoutes.BASE_URL)
+@Produces("application/json")
+public class ReceiveMailOverWebRoutes implements Routes {
+
+ public static final String BASE_URL = "/mail-transfer-service";
+
+ private MailQueue queue;
+
+ @Override
+ public String getBasePath() {
+ return BASE_URL;
+ }
+
+ @Inject
+ public ReceiveMailOverWebRoutes(MailQueueFactory<?> queueFactory) {
+ queue = queueFactory.createQueue(MailQueueFactory.SPOOL);
+ }
+
+ @Override
+ public void define(Service service) {
+ defineReceiveMailFromWebService(service);
+ }
+
+ @POST
+ @Path("/mail-transfer-service")
+ @ApiOperation(value = "Receiving a message/rfc822 over REST interface")
+ @ApiImplicitParams({
+ @ApiImplicitParam(required = true, dataType = "string", name = "username", paramType = "path")
+ })
+ @ApiResponses(value = {
+ @ApiResponse(code = HttpStatus.CREATED_201, message = ""),
+ @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Could not ceate mail from supplied body"),
+ @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500,
+ message = "Internal server error - Something went bad on the server side.")
+ })
+ public void defineReceiveMailFromWebService(Service service) {
+ service.post(BASE_URL, (request, response) -> {
+ //parse MimeMessage from request body
+ MimeMessage mimeMessage = new MimeMessage(Session.getDefaultInstance(System.getProperties()), new ByteArrayInputStream(request.bodyAsBytes()));
+ //create MailImpl object from MimeMessage
+ MailImpl mail = MailImpl.fromMimeMessage(UUID.randomUUID().toString(), mimeMessage);
Review comment:
If we are fine about never having a different envelope that the mime headers, then that's fine with me!
##########
File path: server/protocols/webadmin/webadmin-mail-over-web/src/main/java/org/apache/james/webadmin/routes/ReceiveMailOverWebRoutes.java
##########
@@ -0,0 +1,80 @@
+package org.apache.james.webadmin.routes;
+
+import java.io.ByteArrayInputStream;
+import java.util.UUID;
+
+import javax.inject.Inject;
+import javax.mail.Session;
+import javax.mail.internet.MimeMessage;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+
+import org.apache.james.queue.api.MailQueue;
+import org.apache.james.queue.api.MailQueueFactory;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractor;
+import org.eclipse.jetty.http.HttpStatus;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiImplicitParam;
+import io.swagger.annotations.ApiImplicitParams;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import spark.Service;
+
+@Api(tags = "OverWebMailReceiver")
+@Path(ReceiveMailOverWebRoutes.BASE_URL)
Review comment:
I prefer a terminology enforcing the process (transfer) rather than on which side of the transfer we are (receive here)
Hence I propose to name this `TransferEmailRoutes`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org