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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
