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 71517901f49091b6cef72bb24c5ecbecd2849d0c Author: Benoit Tellier <[email protected]> AuthorDate: Thu Dec 12 08:18:37 2019 +0100 JAMES-3006 Use Task factory in deleted message vault routes --- .../vault/routes/DeletedMessagesVaultRoutes.java | 200 +++++---------------- .../routes/DeletedMessagesVaultRoutesTest.java | 22 ++- 2 files changed, 58 insertions(+), 164 deletions(-) diff --git a/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/main/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutes.java b/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/main/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutes.java index 81ed6d0..0197c1d 100644 --- a/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/main/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutes.java +++ b/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/main/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutes.java @@ -21,10 +21,7 @@ package org.apache.james.webadmin.vault.routes; import static org.apache.james.webadmin.Constants.SEPARATOR; -import java.util.List; import java.util.Optional; -import java.util.function.Function; -import java.util.stream.Stream; import javax.inject.Inject; import javax.mail.internet.AddressException; @@ -33,13 +30,11 @@ import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.Produces; -import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.StringUtils; import org.apache.james.core.MailAddress; import org.apache.james.core.Username; 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.user.api.UsersRepository; import org.apache.james.user.api.UsersRepositoryException; @@ -49,93 +44,35 @@ import org.apache.james.vault.dto.query.QueryTranslator; import org.apache.james.vault.search.Query; import org.apache.james.webadmin.Constants; 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.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.JsonExtractException; import org.apache.james.webadmin.utils.JsonExtractor; import org.apache.james.webadmin.utils.JsonTransformer; import org.eclipse.jetty.http.HttpStatus; -import com.github.steveash.guavate.Guavate; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; -import com.google.common.base.Preconditions; + 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.HaltException; import spark.Request; -import spark.Response; +import spark.Route; import spark.Service; @Api(tags = "Deleted Messages Vault") @Path(DeletedMessagesVaultRoutes.ROOT_PATH) @Produces(Constants.JSON_CONTENT_TYPE) public class DeletedMessagesVaultRoutes implements Routes { - - enum VaultAction { - RESTORE("restore"), - EXPORT("export"); - - static Optional<VaultAction> getAction(String value) { - Preconditions.checkNotNull(value, "action cannot be null"); - Preconditions.checkArgument(StringUtils.isNotBlank(value), "action cannot be empty or blank"); - - return Stream.of(values()) - .filter(action -> action.value.equals(value)) - .findFirst(); - } - - @VisibleForTesting - static List<String> plainValues() { - return Stream.of(values()) - .map(VaultAction::getValue) - .collect(Guavate.toImmutableList()); - } - - private final String value; - - VaultAction(String value) { - this.value = value; - } - - public String getValue() { - return value; - } - } - - enum VaultScope { - EXPIRED("expired"); - - static Optional<VaultScope> getScope(String value) { - Preconditions.checkNotNull(value, "scope cannot be null"); - Preconditions.checkArgument(StringUtils.isNotBlank(value), "scope cannot be empty or blank"); - - return Stream.of(values()) - .filter(action -> action.value.equals(value)) - .findFirst(); - } - - @VisibleForTesting - static List<String> plainValues() { - return Stream.of(values()) - .map(VaultScope::getValue) - .collect(Guavate.toImmutableList()); - } - - private final String value; - - VaultScope(String value) { - this.value = value; - } - - public String getValue() { - return value; - } - } + private static final TaskRegistrationKey EXPORT_REGISTRATION_KEY = TaskRegistrationKey.of("export"); + private static final TaskRegistrationKey RESTORE_REGISTRATION_KEY = TaskRegistrationKey.of("restore"); + private static final TaskRegistrationKey EXPIRED_REGISTRATION_KEY = TaskRegistrationKey.of("expired"); public static final String ROOT_PATH = "deletedMessages"; public static final String USERS = "users"; @@ -145,7 +82,6 @@ public class DeletedMessagesVaultRoutes implements Routes { static final String USER_PATH = ROOT_PATH + SEPARATOR + USERS + SEPARATOR + USER_PATH_PARAM; private static final String DELETE_PATH = ROOT_PATH + SEPARATOR + USERS + SEPARATOR + USER_PATH_PARAM + SEPARATOR + MESSAGE_PATH_PARAM + SEPARATOR + MESSAGE_ID_PARAM; private static final String SCOPE_QUERY_PARAM = "scope"; - private static final String ACTION_QUERY_PARAM = "action"; private static final String EXPORT_TO_QUERY_PARAM = "exportTo"; private final RestoreService vaultRestore; @@ -181,9 +117,11 @@ public class DeletedMessagesVaultRoutes implements Routes { @Override public void define(Service service) { - service.post(USER_PATH, this::userActions, jsonTransformer); - service.delete(ROOT_PATH, this::deleteWithScope, jsonTransformer); - service.delete(DELETE_PATH, this::deleteMessage, jsonTransformer); + service.post(USER_PATH, userActions(), jsonTransformer); + service.delete(ROOT_PATH, deleteWithScope(), jsonTransformer); + + TaskGenerator deleteTaskGenerator = this::deleteMessage; + service.delete(DELETE_PATH, deleteTaskGenerator.asRoute(taskManager), jsonTransformer); } @POST @@ -220,13 +158,23 @@ public class DeletedMessagesVaultRoutes implements Routes { @ApiResponse(code = HttpStatus.NOT_FOUND_404, message = "Not found - requested user does not exist"), @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500, message = "Internal server error - Something went bad on the server side.") }) - private TaskIdDto userActions(Request request, Response response) throws JsonExtractException { - VaultAction requestedAction = extractParam(request, ACTION_QUERY_PARAM, this::getVaultAction); - validateAction(requestedAction, VaultAction.RESTORE, VaultAction.EXPORT); + private Route userActions() { + return TaskFactory.builder() + .register(EXPORT_REGISTRATION_KEY, this::export) + .register(RESTORE_REGISTRATION_KEY, this::restore) + .buildAsRoute(taskManager); + } - Task requestedTask = generateUserTask(requestedAction, request); - TaskId taskId = taskManager.submit(requestedTask); - return TaskIdDto.respond(response, taskId); + private Task export(Request request) throws JsonExtractException { + Username username = extractUser(request); + validateUserExist(username); + return new DeletedMessagesVaultExportTask(vaultExport, username, extractQuery(request), extractMailAddress(request)); + } + + private Task restore(Request request) throws JsonExtractException { + Username username = extractUser(request); + validateUserExist(username); + return new DeletedMessagesVaultRestoreTask(vaultRestore, username, extractQuery(request)); } @DELETE @@ -234,7 +182,7 @@ public class DeletedMessagesVaultRoutes implements Routes { @ApiImplicitParams({ @ApiImplicitParam( required = true, - name = "action", + name = "scope", dataType = "String", paramType = "query", example = "?scope=expired", @@ -245,10 +193,11 @@ public class DeletedMessagesVaultRoutes implements Routes { @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Bad request - action is invalid"), @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500, message = "Internal server error - Something went bad on the server side.") }) - private TaskIdDto deleteWithScope(Request request, Response response) { - Task vaultTask = generateVaultScopeTask(request); - TaskId taskId = taskManager.submit(vaultTask); - return TaskIdDto.respond(response, taskId); + private Route deleteWithScope() { + return TaskFactory.builder() + .parameterName(SCOPE_QUERY_PARAM) + .register(EXPIRED_REGISTRATION_KEY, request -> deletedMessageVault.deleteExpiredMessagesTask()) + .buildAsRoute(taskManager); } @DELETE @@ -277,42 +226,12 @@ public class DeletedMessagesVaultRoutes implements Routes { @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Bad request - messageId param is invalid"), @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500, message = "Internal server error - Something went bad on the server side.") }) - private TaskIdDto deleteMessage(Request request, Response response) { + private Task deleteMessage(Request request) { Username username = extractUser(request); validateUserExist(username); MessageId messageId = parseMessageId(request); - TaskId taskId = taskManager.submit(new DeletedMessagesVaultDeleteTask(deletedMessageVault, username, messageId)); - return TaskIdDto.respond(response, taskId); - } - - private Task generateVaultScopeTask(Request request) { - VaultScope scope = extractParam(request, SCOPE_QUERY_PARAM, this::getVaultScope); - if (!scope.equals(VaultScope.EXPIRED)) { - throw ErrorResponder.builder() - .statusCode(HttpStatus.BAD_REQUEST_400) - .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) - .message(String.format("'%s' is not a valid scope. Supported values are: %s", - scope.value, - VaultScope.plainValues())) - .haltError(); - } - return deletedMessageVault.deleteExpiredMessagesTask(); - } - - private Task generateUserTask(VaultAction requestedAction, Request request) throws JsonExtractException { - Username username = extractUser(request); - validateUserExist(username); - Query query = translate(jsonExtractor.parse(request.body())); - - switch (requestedAction) { - case RESTORE: - return new DeletedMessagesVaultRestoreTask(vaultRestore, username, query); - case EXPORT: - return new DeletedMessagesVaultExportTask(vaultExport, username, query, extractMailAddress(request)); - default: - throw new NotImplementedException(requestedAction + " is not yet handled."); - } + return new DeletedMessagesVaultDeleteTask(deletedMessageVault, username, messageId); } private void validateUserExist(Username username) { @@ -358,8 +277,9 @@ public class DeletedMessagesVaultRoutes implements Routes { } } - private Query translate(QueryElement queryElement) { + private Query extractQuery(Request request) throws JsonExtractException { try { + QueryElement queryElement = jsonExtractor.parse(request.body()); return queryTranslator.translate(queryElement); } catch (QueryTranslator.QueryTranslatorException e) { throw ErrorResponder.builder() @@ -384,46 +304,6 @@ public class DeletedMessagesVaultRoutes implements Routes { } } - private <T> T extractParam(Request request, String queryParam, Function<String, T> mapper) { - String param = request.queryParams(queryParam); - return Optional.ofNullable(param) - .map(mapper) - .orElseThrow(() -> new IllegalArgumentException("parameter is missing")); - } - - private VaultAction getVaultAction(String actionString) { - return VaultAction.getAction(actionString) - .orElseThrow(() -> new IllegalArgumentException(String.format("'%s' is not a valid action. Supported values are: (%s)", - actionString, - Joiner.on(",").join(VaultAction.plainValues())))); - } - - private VaultScope getVaultScope(String scopeString) { - return VaultScope.getScope(scopeString) - .orElseThrow(() -> new IllegalArgumentException(String.format("'%s' is not a valid scope. Supported values are: (%s)", - scopeString, - Joiner.on(",").join(VaultScope.plainValues())))); - } - - private void validateAction(VaultAction requestedAction, VaultAction... validActions) { - Stream.of(validActions) - .filter(action -> action.equals(requestedAction)) - .findFirst() - .orElseThrow(() -> throwNotSupportedAction(requestedAction, validActions)); - } - - private HaltException throwNotSupportedAction(VaultAction requestAction, VaultAction... validActions) { - return ErrorResponder.builder() - .statusCode(HttpStatus.BAD_REQUEST_400) - .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) - .message(String.format("'%s' is not a valid action. Supported values are: %s", - requestAction.getValue(), - Joiner.on(", ").join(Stream.of(validActions) - .map(action -> String.format("'%s'", action.getValue())) - .collect(Guavate.toImmutableList())))) - .haltError(); - } - private MessageId parseMessageId(Request request) { String messageIdAsString = request.params(MESSAGE_ID_PARAM); try { diff --git a/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/test/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/test/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutesTest.java index 4025a5b..02528fd 100644 --- a/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/test/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutesTest.java +++ b/server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/test/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutesTest.java @@ -290,7 +290,7 @@ class DeletedMessagesVaultRoutesTest { .body("statusCode", is(400)) .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType())) .body("message", is("Invalid arguments supplied in the user request")) - .body("details", is("parameter is missing")); + .body("details", is("'scope' query parameter is compulsory. Supported values are [expired]")); } @Test @@ -304,7 +304,21 @@ class DeletedMessagesVaultRoutesTest { .body("statusCode", is(400)) .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType())) .body("message", is("Invalid arguments supplied in the user request")) - .body("details", is("scope cannot be empty or blank")); + .body("details", is("'scope' query parameter cannot be empty or blank. Supported values are [expired]")); + } + + @Test + void purgeAPIShouldReturnInvalidWhenPassingBlankScope() { + given() + .queryParam("scope", " ") + .when() + .delete() + .then() + .statusCode(HttpStatus.BAD_REQUEST_400) + .body("statusCode", is(400)) + .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType())) + .body("message", is("Invalid arguments supplied in the user request")) + .body("details", is("'scope' query parameter cannot be empty or blank. Supported values are [expired]")); } @Test @@ -318,7 +332,7 @@ class DeletedMessagesVaultRoutesTest { .body("statusCode", is(400)) .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType())) .body("message", is("Invalid arguments supplied in the user request")) - .body("details", startsWith("'invalid action' is not a valid scope.")); + .body("details", startsWith("Invalid value supplied for 'scope': invalid action. Supported values are [expired]")); } @Test @@ -332,7 +346,7 @@ class DeletedMessagesVaultRoutesTest { .body("statusCode", is(400)) .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType())) .body("message", is("Invalid arguments supplied in the user request")) - .body("details", startsWith("'EXPIRED' is not a valid scope.")); + .body("details", is("Invalid value supplied for 'scope': EXPIRED. Supported values are [expired]")); } @ParameterizedTest --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
