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/12/17 01:55:34 UTC

[james-project] 13/24: JAMES-3006 Use Task factory in reindexing routes

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 93a560d58f20dabef4c9a2876d4520e53f45fd93
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu Dec 12 08:14:28 2019 +0100

    JAMES-3006 Use Task factory in reindexing routes
---
 .../webadmin/routes/MessageIdReindexingRoutes.java | 28 +++----
 .../webadmin/routes/ReIndexingRoutesUtil.java      | 38 ----------
 .../james/webadmin/routes/ReindexingRoutes.java    | 87 ++++++++++++----------
 .../webadmin/routes/ReindexingRoutesTest.java      | 39 ++++++----
 4 files changed, 86 insertions(+), 106 deletions(-)

diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/MessageIdReindexingRoutes.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/MessageIdReindexingRoutes.java
index d98206f..bc28e9a 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/MessageIdReindexingRoutes.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/MessageIdReindexingRoutes.java
@@ -19,6 +19,8 @@
 
 package org.apache.james.webadmin.routes;
 
+import static org.apache.james.webadmin.routes.ReindexingRoutes.TASK_PARAMETER;
+
 import javax.inject.Inject;
 import javax.ws.rs.POST;
 import javax.ws.rs.Path;
@@ -26,17 +28,14 @@ import javax.ws.rs.Produces;
 
 import org.apache.james.mailbox.indexer.MessageIdReIndexer;
 import org.apache.james.mailbox.model.MessageId;
-import org.apache.james.task.Task;
-import org.apache.james.task.TaskId;
 import org.apache.james.task.TaskManager;
 import org.apache.james.webadmin.Routes;
-import org.apache.james.webadmin.dto.TaskIdDto;
+import org.apache.james.webadmin.tasks.TaskFactory;
+import org.apache.james.webadmin.tasks.TaskIdDto;
 import org.apache.james.webadmin.utils.ErrorResponder;
 import org.apache.james.webadmin.utils.JsonTransformer;
 import org.eclipse.jetty.http.HttpStatus;
 
-import com.github.fge.lambdas.supplier.ThrowingSupplier;
-
 import io.swagger.annotations.Api;
 import io.swagger.annotations.ApiImplicitParam;
 import io.swagger.annotations.ApiImplicitParams;
@@ -44,7 +43,7 @@ import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 import spark.Request;
-import spark.Response;
+import spark.Route;
 import spark.Service;
 
 @Api(tags = "MessageIdReIndexing")
@@ -75,7 +74,7 @@ public class MessageIdReindexingRoutes implements Routes {
 
     @Override
     public void define(Service service) {
-        service.post(MESSAGE_PATH, this::reIndexMessage, jsonTransformer);
+        service.post(MESSAGE_PATH, reIndexMessage(), jsonTransformer);
     }
 
     @POST
@@ -103,8 +102,11 @@ public class MessageIdReindexingRoutes implements Routes {
         @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500, message = "Internal server error - Something went bad on the server side."),
         @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Bad request - details in the returned error message")
     })
-    private TaskIdDto reIndexMessage(Request request, Response response) {
-        return wrap(request, response, () -> reIndexer.reIndex(extractMessageId(request)));
+    private Route reIndexMessage() {
+        return TaskFactory.builder()
+            .parameterName(TASK_PARAMETER)
+            .register(ReindexingRoutes.RE_INDEX, request -> reIndexer.reIndex(extractMessageId(request)))
+            .buildAsRoute(taskManager);
     }
 
     private MessageId extractMessageId(Request request) {
@@ -119,12 +121,4 @@ public class MessageIdReindexingRoutes implements Routes {
                 .haltError();
         }
     }
-
-    private TaskIdDto wrap(Request request, Response response, ThrowingSupplier<Task> taskGenerator) {
-        ReIndexingRoutesUtil.enforceTaskParameter(request);
-
-        Task task = taskGenerator.get();
-        TaskId taskId = taskManager.submit(task);
-        return TaskIdDto.respond(response, taskId);
-    }
 }
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/ReIndexingRoutesUtil.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/ReIndexingRoutesUtil.java
deleted file mode 100644
index c82f055..0000000
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/ReIndexingRoutesUtil.java
+++ /dev/null
@@ -1,38 +0,0 @@
-/****************************************************************
- * 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.webadmin.routes;
-
-import org.apache.james.webadmin.utils.ErrorResponder;
-import org.eclipse.jetty.http.HttpStatus;
-
-import spark.Request;
-
-class ReIndexingRoutesUtil {
-    static void enforceTaskParameter(Request request) {
-        String task = request.queryParams("task");
-        if (!"reIndex".equals(task)) {
-            throw ErrorResponder.builder()
-                .statusCode(HttpStatus.BAD_REQUEST_400)
-                .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
-                .message("task query parameter is mandatory. The only supported value is `reIndex`")
-                .haltError();
-        }
-    }
-}
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/ReindexingRoutes.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/ReindexingRoutes.java
index d838339..75f0cdd 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/ReindexingRoutes.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/ReindexingRoutes.java
@@ -36,8 +36,11 @@ import org.apache.james.task.TaskId;
 import org.apache.james.task.TaskManager;
 import org.apache.james.task.TaskNotFoundException;
 import org.apache.james.webadmin.Routes;
-import org.apache.james.webadmin.dto.TaskIdDto;
 import org.apache.james.webadmin.service.PreviousReIndexingService;
+import org.apache.james.webadmin.tasks.TaskFactory;
+import org.apache.james.webadmin.tasks.TaskGenerator;
+import org.apache.james.webadmin.tasks.TaskIdDto;
+import org.apache.james.webadmin.tasks.TaskRegistrationKey;
 import org.apache.james.webadmin.utils.ErrorResponder;
 import org.apache.james.webadmin.utils.JsonTransformer;
 import org.eclipse.jetty.http.HttpStatus;
@@ -50,18 +53,16 @@ import io.swagger.annotations.ApiImplicitParams;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
+import spark.HaltException;
 import spark.Request;
-import spark.Response;
+import spark.Route;
 import spark.Service;
 
 @Api(tags = "ReIndexing (mailboxes)")
 @Path("/mailboxes")
 @Produces("application/json")
 public class ReindexingRoutes implements Routes {
-    @FunctionalInterface
-    interface TaskGenerator {
-        Task generate() throws MailboxException;
-    }
+
 
     private static final String BASE_PATH = "/mailboxes";
     private static final String USER_QUERY_PARAM = "user";
@@ -70,6 +71,8 @@ public class ReindexingRoutes implements Routes {
     private static final String UID_PARAM = ":uid";
     private static final String MAILBOX_PATH = BASE_PATH + "/" + MAILBOX_PARAM;
     private static final String MESSAGE_PATH = MAILBOX_PATH + "/mails/" + UID_PARAM;
+    static final TaskRegistrationKey RE_INDEX = TaskRegistrationKey.of("reIndex");
+    static final String TASK_PARAMETER = "task";
 
     private final TaskManager taskManager;
     private final PreviousReIndexingService previousReIndexingService;
@@ -93,9 +96,9 @@ public class ReindexingRoutes implements Routes {
 
     @Override
     public void define(Service service) {
-        service.post(BASE_PATH, this::reIndexAll, jsonTransformer);
-        service.post(MAILBOX_PATH, this::reIndexMailbox, jsonTransformer);
-        service.post(MESSAGE_PATH, this::reIndexMessage, jsonTransformer);
+        service.post(BASE_PATH, reIndexAll(), jsonTransformer);
+        service.post(MAILBOX_PATH, reIndexMailbox(), jsonTransformer);
+        service.post(MESSAGE_PATH, reIndexMessage(), jsonTransformer);
     }
 
     @POST
@@ -131,20 +134,27 @@ public class ReindexingRoutes implements Routes {
         @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500, message = "Internal server error - Something went bad on the server side."),
         @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Bad request - details in the returned error message")
     })
-    private TaskIdDto reIndexAll(Request request, Response response) {
+    private Route reIndexAll() {
+        return TaskFactory.builder()
+            .parameterName(TASK_PARAMETER)
+            .register(RE_INDEX, wrap(this::reIndexAll))
+            .buildAsRoute(taskManager);
+    }
+
+    private Task reIndexAll(Request request) throws MailboxException {
         boolean userReIndexing = !Strings.isNullOrEmpty(request.queryParams(USER_QUERY_PARAM));
         boolean indexingCorrection = !Strings.isNullOrEmpty(request.queryParams(RE_INDEX_FAILED_MESSAGES_QUERY_PARAM));
         if (userReIndexing && indexingCorrection) {
-            return rejectInvalidQueryParameterCombination();
+            throw rejectInvalidQueryParameterCombination();
         }
         if (userReIndexing) {
-            return wrap(request, response, () -> reIndexer.reIndex(extractUser(request)));
+            return reIndexer.reIndex(extractUser(request));
         }
         if (indexingCorrection) {
             IndexingDetailInformation indexingDetailInformation = retrieveIndexingExecutionDetails(request);
-            return wrap(request, response, () -> reIndexer.reIndex(indexingDetailInformation.failures()));
+            return reIndexer.reIndex(indexingDetailInformation.failures());
         }
-        return wrap(request, response, reIndexer::reIndex);
+        return reIndexer.reIndex();
     }
 
     private IndexingDetailInformation retrieveIndexingExecutionDetails(Request request) {
@@ -182,8 +192,8 @@ public class ReindexingRoutes implements Routes {
         }
     }
 
-    private TaskIdDto rejectInvalidQueryParameterCombination() {
-        throw ErrorResponder.builder()
+    private HaltException rejectInvalidQueryParameterCombination() {
+        return ErrorResponder.builder()
             .statusCode(HttpStatus.BAD_REQUEST_400)
             .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
             .message("Can not specify '" + USER_QUERY_PARAM + "' and '" + RE_INDEX_FAILED_MESSAGES_QUERY_PARAM + "' query parameters at the same time")
@@ -223,9 +233,11 @@ public class ReindexingRoutes implements Routes {
         @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500, message = "Internal server error - Something went bad on the server side."),
         @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Bad request - details in the returned error message")
     })
-    private TaskIdDto reIndexMailbox(Request request, Response response) {
-        return wrap(request, response,
-            () -> reIndexer.reIndex(extractMailboxId(request)));
+    private Route reIndexMailbox() {
+        return TaskFactory.builder()
+            .parameterName(TASK_PARAMETER)
+            .register(RE_INDEX, wrap(request -> reIndexer.reIndex(extractMailboxId(request))))
+            .buildAsRoute(taskManager);
     }
 
     @POST
@@ -263,27 +275,26 @@ public class ReindexingRoutes implements Routes {
             defaultValue = "none",
             value = "Compulsory. Needs to be a valid UID")
     })
-    private TaskIdDto reIndexMessage(Request request, Response response) {
-        return wrap(request, response,
-            () -> reIndexer.reIndex(extractMailboxId(request), extractUid(request)));
+    private Route reIndexMessage() {
+        return TaskFactory.builder()
+            .parameterName(TASK_PARAMETER)
+            .register(RE_INDEX, wrap(request -> reIndexer.reIndex(extractMailboxId(request), extractUid(request))))
+            .buildAsRoute(taskManager);
     }
 
-    private TaskIdDto wrap(Request request, Response response, TaskGenerator taskGenerator) {
-        ReIndexingRoutesUtil.enforceTaskParameter(request);
-        try {
-            Task task = taskGenerator.generate();
-            TaskId taskId = taskManager.submit(task);
-            return TaskIdDto.respond(response, taskId);
-        } catch (MailboxNotFoundException e) {
-            throw ErrorResponder.builder()
-                .statusCode(HttpStatus.NOT_FOUND_404)
-                .type(ErrorResponder.ErrorType.NOT_FOUND)
-                .message("mailbox not found")
-                .cause(e)
-                .haltError();
-        } catch (MailboxException e) {
-            throw new RuntimeException(e);
-        }
+    private TaskGenerator wrap(TaskGenerator toBeWrapped) {
+        return request -> {
+            try {
+                return toBeWrapped.generate(request);
+            } catch (MailboxNotFoundException e) {
+                throw ErrorResponder.builder()
+                    .statusCode(HttpStatus.NOT_FOUND_404)
+                    .type(ErrorResponder.ErrorType.NOT_FOUND)
+                    .message("mailbox not found")
+                    .cause(e)
+                    .haltError();
+            }
+        };
     }
 
     private Username extractUser(Request request) {
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/ReindexingRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/ReindexingRoutesTest.java
index 6271909..f7b94e3 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/ReindexingRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/ReindexingRoutesTest.java
@@ -130,7 +130,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'task' query parameter is compulsory. Supported values are [reIndex]"));
             }
 
             @Test
@@ -141,7 +142,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Invalid value supplied for 'task': bad. Supported values are [reIndex]"));
             }
         }
 
@@ -286,7 +288,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'task' query parameter is compulsory. Supported values are [reIndex]"));
             }
 
             @Test
@@ -300,7 +303,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Invalid value supplied for 'task': bad. Supported values are [reIndex]"));
             }
 
             @Test
@@ -474,7 +478,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'task' query parameter is compulsory. Supported values are [reIndex]"));
             }
 
             @Test
@@ -488,7 +493,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Invalid value supplied for 'task': bad. Supported values are [reIndex]"));
             }
 
             @Test
@@ -662,7 +668,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'task' query parameter is compulsory. Supported values are [reIndex]"));
             }
 
             @Test
@@ -676,7 +683,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Invalid value supplied for 'task': bad. Supported values are [reIndex]"));
             }
 
             @Test
@@ -821,7 +829,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'task' query parameter is compulsory. Supported values are [reIndex]"));
             }
 
             @Test
@@ -832,7 +841,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Invalid value supplied for 'task': bad. Supported values are [reIndex]"));
             }
 
             @Test
@@ -961,7 +971,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'task' query parameter is compulsory. Supported values are [reIndex]"));
             }
 
             @Test
@@ -1007,7 +1018,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("task query parameter is mandatory. The only supported value is `reIndex`"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Invalid value supplied for 'task': bad. Supported values are [reIndex]"));
             }
 
             @Test
@@ -1022,7 +1034,8 @@ class ReindexingRoutesTest {
                     .statusCode(HttpStatus.BAD_REQUEST_400)
                     .body("statusCode", is(400))
                     .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
-                    .body("message", is("TaskId bbdb69c9-082a-44b0-a85a-6e33e74287a5 does not exist"));
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Invalid value supplied for 'task': bad. Supported values are [reIndex]"));
             }
         }
 


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