JAMES-2233 WebAdmin responses more details when error
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/bde6bdb7 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/bde6bdb7 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/bde6bdb7 Branch: refs/heads/master Commit: bde6bdb7d4921af4245cd7ab84e6aa3943cbc517 Parents: 16410ae Author: quynhn <[email protected]> Authored: Fri Dec 1 11:51:19 2017 +0700 Committer: benwa <[email protected]> Committed: Wed Dec 6 09:10:07 2017 +0700 ---------------------------------------------------------------------- .../routes/CassandraMigrationRoutes.java | 45 ++- .../routes/CassandraMigrationRoutesTest.java | 45 ++- server/protocols/webadmin/webadmin-core/pom.xml | 5 + .../james/webadmin/utils/ErrorResponder.java | 145 ++++++++ .../webadmin/utils/ErrorResponderTest.java | 139 ++++++++ .../james/webadmin/routes/DomainsRoutes.java | 43 ++- .../james/webadmin/routes/GroupsRoutes.java | 34 +- .../james/webadmin/routes/UserRoutes.java | 40 ++- .../webadmin/routes/DomainsRoutesTest.java | 30 +- .../james/webadmin/routes/GroupsRoutesTest.java | 166 ++++++++-- .../webadmin/routes/GlobalQuotaRoutes.java | 37 ++- .../webadmin/routes/UserMailboxesRoutes.java | 77 ++++- .../webadmin/routes/GlobalQuotaRoutesTest.java | 61 +++- .../routes/UserMailboxesRoutesTest.java | 330 +++++++++++++++---- 14 files changed, 1029 insertions(+), 168 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/CassandraMigrationRoutes.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/CassandraMigrationRoutes.java b/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/CassandraMigrationRoutes.java index c574f13..ff08bea 100644 --- a/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/CassandraMigrationRoutes.java +++ b/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/CassandraMigrationRoutes.java @@ -26,7 +26,10 @@ import org.apache.james.webadmin.Routes; import org.apache.james.webadmin.dto.CassandraVersionRequest; import org.apache.james.webadmin.service.CassandraMigrationService; import org.apache.james.webadmin.service.MigrationException; +import org.apache.james.webadmin.utils.ErrorResponder; +import org.apache.james.webadmin.utils.ErrorResponder.ErrorType; import org.apache.james.webadmin.utils.JsonTransformer; +import org.eclipse.jetty.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,9 +44,6 @@ public class CassandraMigrationRoutes implements Routes { private static final String VERSION_UPGRADE_BASE = VERSION_BASE + "/upgrade"; private static final String VERSION_UPGRADE_TO_LATEST_BASE = VERSION_UPGRADE_BASE + "/latest"; private static final int NO_CONTENT = 204; - private static final int INVALID_VERSION = 400; - private static final int MIGRATION_CAN_NOT_BE_PERFORMED = 410; - private static final int INTERNAL_ERROR = 500; private final CassandraMigrationService cassandraMigrationService; private final JsonTransformer jsonTransformer; @@ -72,15 +72,28 @@ public class CassandraMigrationRoutes implements Routes { response.status(NO_CONTENT); } catch (NullPointerException | IllegalArgumentException e) { LOGGER.info("Invalid request for version upgrade"); - response.status(INVALID_VERSION); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid request for version upgrade") + .cause(e) + .haltError(); } catch (IllegalStateException e) { LOGGER.info("The migration requested can not be performed.", e); - response.status(MIGRATION_CAN_NOT_BE_PERFORMED); - response.body(e.getMessage()); + throw ErrorResponder.builder() + .statusCode(HttpStatus.GONE_410) + .type(ErrorType.WRONG_STATE) + .message("The migration requested can not be performed") + .cause(e) + .haltError(); } catch (MigrationException e) { LOGGER.error("An error lead to partial migration process", e); - response.status(INTERNAL_ERROR); - response.body(e.getMessage()); + throw ErrorResponder.builder() + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .type(ErrorType.SERVER_ERROR) + .message("An error lead to partial migration process") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; }); @@ -90,12 +103,20 @@ public class CassandraMigrationRoutes implements Routes { cassandraMigrationService.upgradeToLastVersion(); } catch (IllegalStateException e) { LOGGER.info("The migration requested can not be performed.", e); - response.status(MIGRATION_CAN_NOT_BE_PERFORMED); - response.body(e.getMessage()); + throw ErrorResponder.builder() + .statusCode(HttpStatus.GONE_410) + .type(ErrorType.WRONG_STATE) + .message("The migration requested can not be performed") + .cause(e) + .haltError(); } catch (MigrationException e) { LOGGER.error("An error lead to partial migration process", e); - response.status(INTERNAL_ERROR); - response.body(e.getMessage()); + throw ErrorResponder.builder() + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .type(ErrorType.SERVER_ERROR) + .message("An error lead to partial migration process") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/CassandraMigrationRoutesTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/CassandraMigrationRoutesTest.java b/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/CassandraMigrationRoutesTest.java index e647011..a6d6c14 100644 --- a/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/CassandraMigrationRoutesTest.java +++ b/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/CassandraMigrationRoutesTest.java @@ -145,12 +145,23 @@ public class CassandraMigrationRoutesTest { public void postShouldReturnErrorCodeWhenInvalidVersion() throws Exception { when(schemaVersionDAO.getCurrentSchemaVersion()).thenReturn(CompletableFuture.completedFuture(Optional.of(OLDER_VERSION))); - given() + Map<String, Object> errors = given() .body(String.valueOf("NonInt")) .with() .post("/upgrade") .then() - .statusCode(400); + .statusCode(400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Invalid request for version upgrade") + .containsEntry("cause", "For input string: \"NonInt\""); verifyNoMoreInteractions(schemaVersionDAO); } @@ -175,12 +186,23 @@ public class CassandraMigrationRoutesTest { public void postShouldNotDoMigrationWhenCurrentVersionIsNewerThan() throws Exception { when(schemaVersionDAO.getCurrentSchemaVersion()).thenReturn(CompletableFuture.completedFuture(Optional.of(CURRENT_VERSION))); - given() + Map<String, Object> errors = given() .body(String.valueOf(OLDER_VERSION)) .with() .post("/upgrade") .then() - .statusCode(410); + .statusCode(410) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 410) + .containsEntry("type", "WrongState") + .containsEntry("message", "The migration requested can not be performed") + .containsEntry("cause", "Current version is already up to date"); verify(schemaVersionDAO, times(1)).getCurrentSchemaVersion(); verifyNoMoreInteractions(schemaVersionDAO); @@ -205,10 +227,21 @@ public class CassandraMigrationRoutesTest { public void postShouldNotDoMigrationToLatestVersionWhenItIsUpToDate() throws Exception { when(schemaVersionDAO.getCurrentSchemaVersion()).thenReturn(CompletableFuture.completedFuture(Optional.of(LATEST_VERSION))); - when() + Map<String, Object> errors = when() .post("/upgrade/latest") .then() - .statusCode(410); + .statusCode(410) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 410) + .containsEntry("type", "WrongState") + .containsEntry("message", "The migration requested can not be performed") + .containsEntry("cause", "Current version is already up to date"); verify(schemaVersionDAO, times(1)).getCurrentSchemaVersion(); verifyNoMoreInteractions(schemaVersionDAO); http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-core/pom.xml ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-core/pom.xml b/server/protocols/webadmin/webadmin-core/pom.xml index 4d5e1c5..d99b417 100644 --- a/server/protocols/webadmin/webadmin-core/pom.xml +++ b/server/protocols/webadmin/webadmin-core/pom.xml @@ -104,6 +104,11 @@ <scope>test</scope> </dependency> <dependency> + <groupId>com.jayway.jsonpath</groupId> + <artifactId>json-path</artifactId> + <scope>test</scope> + </dependency> + <dependency> <groupId>org.mockito</groupId> <artifactId>mockito-core</artifactId> <scope>test</scope> http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/ErrorResponder.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/ErrorResponder.java b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/ErrorResponder.java new file mode 100644 index 0000000..90642bb --- /dev/null +++ b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/utils/ErrorResponder.java @@ -0,0 +1,145 @@ +/**************************************************************** + * 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.utils; + +import static spark.Spark.halt; + +import java.util.Objects; +import java.util.Optional; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; + +import spark.HaltException; + +public class ErrorResponder { + public enum ErrorType { + INVALID_ARGUMENT("InvalidArgument"), + WRONG_STATE("WrongState"), + SERVER_ERROR("ServerError"); + + private final String type; + + ErrorType(String type) { + this.type = type; + } + + public String getType() { + return type; + } + } + + private Integer statusCode; + private ErrorType type; + private String message; + private Optional<Exception> cause; + + public ErrorResponder() { + cause = Optional.empty(); + } + + public static ErrorResponder builder() { + return new ErrorResponder(); + } + + public ErrorResponder statusCode(int statusCode) { + this.statusCode = statusCode; + return this; + } + + public ErrorResponder type(ErrorType type) { + this.type = type; + return this; + } + + public ErrorResponder message(String message) { + this.message = message; + return this; + } + + public ErrorResponder cause(Exception cause) { + this.cause = Optional.of(cause); + return this; + } + + public HaltException haltError() { + Preconditions.checkNotNull(statusCode, "statusCode must not be null in case of error"); + Preconditions.checkNotNull(type, "type must not be null in case of error"); + Preconditions.checkNotNull(message, "message must not be null in case of error"); + try { + return halt(statusCode, new JsonTransformer().render(new ErrorDetail(statusCode, + type.getType(), + message, + cause.map(e -> Optional.of(e.getMessage())).orElse(Optional.empty())))); + } catch (JsonProcessingException e) { + return halt(statusCode); + } + } + + static class ErrorDetail { + private final int statusCode; + private final String type; + private final String message; + private final Optional<String> cause; + + @VisibleForTesting + ErrorDetail(int statusCode, String type, String message, Optional<String> cause) { + this.statusCode = statusCode; + this.type = type; + this.message = message; + this.cause = cause; + } + + public int getStatusCode() { + return statusCode; + } + + public String getType() { + return type; + } + + public String getMessage() { + return message; + } + + public Optional<String> getCause() { + return cause; + } + + @Override + public final boolean equals(Object o) { + if (o instanceof ErrorDetail) { + ErrorDetail that = (ErrorDetail) o; + + return Objects.equals(this.statusCode, that.statusCode) + && Objects.equals(this.type, that.type) + && Objects.equals(this.message, that.message) + && Objects.equals(this.cause, that.cause); + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(statusCode, type, message, cause); + } + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/utils/ErrorResponderTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/utils/ErrorResponderTest.java b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/utils/ErrorResponderTest.java new file mode 100644 index 0000000..3e98aae --- /dev/null +++ b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/utils/ErrorResponderTest.java @@ -0,0 +1,139 @@ +/**************************************************************** + * 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.utils; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Objects; +import java.util.Optional; + +import org.junit.Test; + +import com.jayway.jsonpath.DocumentContext; +import com.jayway.jsonpath.JsonPath; + +import spark.HaltException; + +public class ErrorResponderTest { + + public static final Optional<String> NO_CAUSE = Optional.empty(); + + @Test + public void haltErrorShouldThrowWhenNoStatusCode() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .haltError()) + .isInstanceOf(NullPointerException.class); + } + + @Test + public void haltErrorShouldThrowWhenNoType() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .statusCode(400) + .haltError()) + .isInstanceOf(NullPointerException.class); + } + + @Test + public void haltErrorShouldThrowWhenNoMessage() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .statusCode(400) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .haltError()) + .isInstanceOf(NullPointerException.class); + } + + @Test + public void haltErrorShouldReturnBodyWithStatusCodeWhenSetting() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .statusCode(404) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .message("Error") + .haltError()) + .isInstanceOf(HaltException.class) + .matches(e -> hasStatus(e, 404)) + .matches(e -> bodyHasErrorDetail(e, new ErrorResponder.ErrorDetail(404, "InvalidArgument", "Error", NO_CAUSE))); + } + + @Test + public void haltErrorShouldReturnBodyWithErrorTypeWhenSetting() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .statusCode(400) + .type(ErrorResponder.ErrorType.WRONG_STATE) + .message("Error") + .haltError()) + .isInstanceOf(HaltException.class) + .matches(e -> hasStatus(e, 400)) + .matches(e -> bodyHasErrorDetail(e, new ErrorResponder.ErrorDetail(400, "WrongState", "Error", NO_CAUSE))); + } + + @Test + public void haltErrorShouldReturnBodyWithErrorMessageWhenSetting() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .statusCode(400) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .message("It has error") + .haltError()) + .isInstanceOf(HaltException.class) + .matches(e -> hasStatus(e, 400)) + .matches(e -> bodyHasErrorDetail(e, new ErrorResponder.ErrorDetail(400, "InvalidArgument", "It has error", NO_CAUSE))); + } + + @Test + public void haltErrorShouldReturnBodyWithCauseTypeWhenSetting() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .statusCode(400) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .message("Error") + .cause(new IllegalArgumentException("The input data is invalid")) + .haltError()) + .isInstanceOf(HaltException.class) + .matches(e -> hasStatus(e, 400)) + .matches(e -> bodyHasErrorDetail(e, new ErrorResponder.ErrorDetail(400, "InvalidArgument", "Error", Optional.of("The input data is invalid")))); + } + + @Test + public void haltErrorShouldReturnBodyWithErrorDetail() throws Exception { + assertThatThrownBy(() -> ErrorResponder.builder() + .statusCode(400) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .message("Error") + .cause(new IllegalArgumentException("The input data is invalid")) + .haltError()) + .isInstanceOf(HaltException.class) + .matches(e -> hasStatus(e, 400)) + .matches(e -> bodyHasErrorDetail(e, new ErrorResponder.ErrorDetail(400, "InvalidArgument", "Error", Optional.of("The input data is invalid")))); + } + + private boolean hasStatus(Throwable throwable, int status) { + HaltException haltException = (HaltException) throwable; + return Objects.equals(haltException.statusCode(), status); + } + + private boolean bodyHasErrorDetail(Throwable throwable, ErrorResponder.ErrorDetail errorDetail) { + HaltException haltException = (HaltException) throwable; + DocumentContext jsonPath = JsonPath.parse(haltException.body()); + return errorDetail.equals(new ErrorResponder.ErrorDetail( + jsonPath.read("$.statusCode"), + jsonPath.read("$.type"), + jsonPath.read("$.message"), + Optional.ofNullable(jsonPath.read("$.cause")))); + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java index 6b6fb9a..0864a8a 100644 --- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java +++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DomainsRoutes.java @@ -22,6 +22,7 @@ package org.apache.james.webadmin.routes; import static org.apache.james.webadmin.Constants.SEPARATOR; import java.util.List; + import javax.inject.Inject; import javax.ws.rs.DELETE; import javax.ws.rs.GET; @@ -33,7 +34,15 @@ import org.apache.james.domainlist.api.DomainList; import org.apache.james.domainlist.api.DomainListException; import org.apache.james.webadmin.Constants; import org.apache.james.webadmin.Routes; +import org.apache.james.webadmin.utils.ErrorResponder; +import org.apache.james.webadmin.utils.ErrorResponder.ErrorType; import org.apache.james.webadmin.utils.JsonTransformer; +import org.eclipse.jetty.http.HttpStatus; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import io.swagger.annotations.Api; import io.swagger.annotations.ApiImplicitParam; @@ -41,12 +50,6 @@ import io.swagger.annotations.ApiImplicitParams; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.base.Preconditions; -import com.google.common.base.Strings; - import spark.Request; import spark.Response; import spark.Service; @@ -160,15 +163,26 @@ public class DomainsRoutes implements Routes { } private String addDomain(Request request, Response response) { + String domainName = request.params(DOMAIN_NAME); try { - addDomain(request.params(DOMAIN_NAME)); + addDomain(domainName); response.status(204); } catch (DomainListException e) { - LOGGER.info("{} already exists", request.params(DOMAIN_NAME)); - response.status(204); + LOGGER.info("{} already exists", domainName); + throw ErrorResponder.builder() + .statusCode(HttpStatus.NO_CONTENT_204) + .type(ErrorType.INVALID_ARGUMENT) + .message(domainName + " already exists") + .cause(e) + .haltError(); } catch (IllegalArgumentException e) { LOGGER.info("Invalid request for domain creation"); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid request for domain creation " + domainName) + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; } @@ -181,8 +195,13 @@ public class DomainsRoutes implements Routes { } private String exists(Request request, Response response) throws DomainListException { - if (!domainList.containsDomain(request.params(DOMAIN_NAME))) { - response.status(404); + String domainName = request.params(DOMAIN_NAME); + if (!domainList.containsDomain(domainName)) { + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("The domain list does not contain: " + domainName) + .haltError(); } else { response.status(204); } http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java index 288a03b..b2c6399 100644 --- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java +++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java @@ -49,6 +49,8 @@ import org.apache.james.user.api.UsersRepositoryException; import org.apache.james.util.streams.Iterators; import org.apache.james.webadmin.Constants; import org.apache.james.webadmin.Routes; +import org.apache.james.webadmin.utils.ErrorResponder; +import org.apache.james.webadmin.utils.ErrorResponder.ErrorType; import org.apache.james.webadmin.utils.JsonExtractException; import org.apache.james.webadmin.utils.JsonTransformer; import org.eclipse.jetty.http.HttpStatus; @@ -158,13 +160,21 @@ public class GroupsRoutes implements Routes { private void ensureRegisteredDomain(String domain) throws DomainListException { if (!domainList.containsDomain(domain)) { - throw halt(HttpStatus.FORBIDDEN_403); + throw ErrorResponder.builder() + .statusCode(HttpStatus.FORBIDDEN_403) + .type(ErrorType.INVALID_ARGUMENT) + .message("Server doesn't own the domain: " + domain) + .haltError(); } } private void ensureNotShadowingAnotherAddress(MailAddress groupAddress) throws UsersRepositoryException { if (usersRepository.contains(groupAddress.asString())) { - throw halt(HttpStatus.CONFLICT_409); + throw ErrorResponder.builder() + .statusCode(HttpStatus.CONFLICT_409) + .type(ErrorType.INVALID_ARGUMENT) + .message("Requested group address is already used for another purpose") + .haltError(); } } @@ -217,16 +227,30 @@ public class GroupsRoutes implements Routes { String decodedAddress = URLDecoder.decode(address, "UTF-8"); return new MailAddress(decodedAddress); } catch (AddressException e) { - throw halt(HttpStatus.BAD_REQUEST_400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("The group is not an email address") + .cause(e) + .haltError(); } catch (UnsupportedEncodingException e) { LOGGER.error("UTF-8 should be a valid encoding"); - throw halt(HttpStatus.INTERNAL_SERVER_ERROR_500); + throw ErrorResponder.builder() + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .type(ErrorType.SERVER_ERROR) + .message("Internal server error - Something went bad on the server side.") + .cause(e) + .haltError(); } } private void ensureNonEmptyMappings(Mappings mappings) { if (mappings == null || mappings.isEmpty()) { - throw halt(HttpStatus.NOT_FOUND_404); + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("The group does not exist") + .haltError(); } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java index 9c73542..e540d96 100644 --- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java +++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java @@ -34,9 +34,14 @@ import org.apache.james.webadmin.Routes; import org.apache.james.webadmin.dto.AddUserRequest; import org.apache.james.webadmin.dto.UserResponse; import org.apache.james.webadmin.service.UserService; +import org.apache.james.webadmin.utils.ErrorResponder; +import org.apache.james.webadmin.utils.ErrorResponder.ErrorType; 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 org.slf4j.Logger; +import org.slf4j.LoggerFactory; import io.swagger.annotations.Api; import io.swagger.annotations.ApiImplicitParam; @@ -44,9 +49,6 @@ import io.swagger.annotations.ApiImplicitParams; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import spark.Request; import spark.Response; import spark.Service; @@ -135,12 +137,20 @@ public class UserRoutes implements Routes { response.status(204); return Constants.EMPTY_BODY; } catch (UsersRepositoryException e) { - response.status(204); - return "The user " + username + " does not exists"; + throw ErrorResponder.builder() + .statusCode(HttpStatus.NO_CONTENT_204) + .type(ErrorType.INVALID_ARGUMENT) + .message("The user " + username + " does not exists") + .cause(e) + .haltError(); } catch (IllegalArgumentException e) { LOGGER.info("Invalid user path", e); - response.status(400); - return Constants.EMPTY_BODY; + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid user path") + .cause(e) + .haltError(); } } @@ -151,12 +161,20 @@ public class UserRoutes implements Routes { response); } catch (JsonExtractException e) { LOGGER.info("Error while deserializing addUser request", e); - response.status(400); - return Constants.EMPTY_BODY; + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Error while deserializing addUser request") + .cause(e) + .haltError(); } catch (IllegalArgumentException e) { LOGGER.info("Invalid user path", e); - response.status(400); - return Constants.EMPTY_BODY; + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid user path") + .cause(e) + .haltError(); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java index 5e825e8..a403b5c 100644 --- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java +++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DomainsRoutesTest.java @@ -27,6 +27,8 @@ import static com.jayway.restassured.config.RestAssuredConfig.newConfig; import static org.apache.james.webadmin.Constants.SEPARATOR; import static org.apache.james.webadmin.WebAdminServer.NO_CONFIGURATION; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.startsWith; import static org.mockito.Matchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -35,6 +37,7 @@ import static org.mockito.Mockito.when; import java.net.InetAddress; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Map; import org.apache.james.dnsservice.api.DNSService; import org.apache.james.domainlist.api.DomainList; @@ -157,10 +160,20 @@ public class DomainsRoutesTest { @Test public void putShouldReturnUserErrorWhenNameContainsAT() { - when() + Map<String, Object> errors = when() .put(DOMAIN + "@" + DOMAIN) .then() - .statusCode(400); + .statusCode(400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Invalid request for domain creation domain@domain"); } @Test @@ -173,12 +186,21 @@ public class DomainsRoutesTest { @Test public void putShouldReturnUserErrorWhenNameIsTooLong() { - when() + Map<String, Object> errors = when() .put(DOMAIN + "0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789." + "0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789." + "0123456789.0123456789.0123456789.") .then() - .statusCode(400); + .statusCode(400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument"); } @Test http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java index 24ace5c..ff357e8 100644 --- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java +++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java @@ -26,6 +26,7 @@ import static com.jayway.restassured.config.RestAssuredConfig.newConfig; import static org.apache.james.webadmin.Constants.SEPARATOR; import static org.apache.james.webadmin.WebAdminServer.NO_CONFIGURATION; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doThrow; @@ -33,6 +34,7 @@ import static org.mockito.Mockito.mock; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Map; import org.apache.james.dnsservice.api.DNSService; import org.apache.james.domainlist.api.DomainList; @@ -59,6 +61,7 @@ import com.jayway.restassured.RestAssured; import com.jayway.restassured.builder.RequestSpecBuilder; import com.jayway.restassured.filter.log.LogDetail; import com.jayway.restassured.http.ContentType; + import de.bechte.junit.runners.context.HierarchicalContextRunner; @RunWith(HierarchicalContextRunner.class) @@ -148,10 +151,20 @@ public class GroupsRoutesTest { @Test public void getUnregisteredGroupShouldReturnNotFound() { - when() + Map<String, Object> errors = when() .get("[email protected]") .then() - .statusCode(HttpStatus.NOT_FOUND_404); + .statusCode(HttpStatus.NOT_FOUND_404) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 404) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "The group does not exist"); } @Test @@ -276,11 +289,20 @@ public class GroupsRoutesTest { @Test public void putUserInGroupShouldNotAllowGroupOnUnregisteredDomain() throws UsersRepositoryException, DomainListException { - when() + Map<String, Object> errors = when() .put("group@unregisteredDomain" + SEPARATOR + USER_A) .then() + .statusCode(HttpStatus.FORBIDDEN_403) .contentType(ContentType.JSON) - .statusCode(HttpStatus.FORBIDDEN_403); + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 403) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Server doesn't own the domain: unregisteredDomain"); } @@ -288,11 +310,20 @@ public class GroupsRoutesTest { public void putUserInGroupShouldNotAllowUserShadowing() throws UsersRepositoryException, DomainListException { usersRepository.addUser(USER_A, "whatever"); - when() + Map<String, Object> errors = when() .put(USER_A + SEPARATOR + USER_B) .then() + .statusCode(HttpStatus.CONFLICT_409) .contentType(ContentType.JSON) - .statusCode(HttpStatus.CONFLICT_409); + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 409) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Requested group address is already used for another purpose"); } @Test @@ -370,18 +401,40 @@ public class GroupsRoutesTest { @Test public void getMalformedGroupShouldReturnBadRequest() { - when() + Map<String, Object> errors = when() .get("not-an-address") .then() - .statusCode(HttpStatus.BAD_REQUEST_400); + .statusCode(HttpStatus.BAD_REQUEST_400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "The group is not an email address") + .containsEntry("cause", "Out of data at position 1 in 'not-an-address'"); } @Test public void putMalformedGroupShouldReturnBadRequest() { - when() + Map<String, Object> errors = when() .put("not-an-address" + SEPARATOR + USER_A) .then() - .statusCode(HttpStatus.BAD_REQUEST_400); + .statusCode(HttpStatus.BAD_REQUEST_400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "The group is not an email address") + .containsEntry("cause", "Out of data at position 1 in 'not-an-address'"); } @Test @@ -389,7 +442,8 @@ public class GroupsRoutesTest { when() .put(GROUP_WITH_SLASH + SEPARATOR + USER_A) .then() - .statusCode(HttpStatus.NOT_FOUND_404); + .statusCode(HttpStatus.NOT_FOUND_404) + .body(containsString("404 Not found")); } @Test @@ -397,15 +451,27 @@ public class GroupsRoutesTest { when() .put(GROUP1 + SEPARATOR + USER_WITH_SLASH) .then() - .statusCode(HttpStatus.NOT_FOUND_404); + .statusCode(HttpStatus.NOT_FOUND_404) + .body(containsString("404 Not found")); } @Test public void putMalformedAddressShouldReturnBadRequest() { - when() + Map<String, Object> errors = when() .put(GROUP1 + SEPARATOR + "not-an-address") .then() - .statusCode(HttpStatus.BAD_REQUEST_400); + .statusCode(HttpStatus.BAD_REQUEST_400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "The group is not an email address") + .containsEntry("cause", "Out of data at position 1 in 'not-an-address'"); } @Test @@ -413,23 +479,46 @@ public class GroupsRoutesTest { when() .put(GROUP1) .then() - .statusCode(HttpStatus.BAD_REQUEST_400); + .statusCode(HttpStatus.BAD_REQUEST_400) + .body(is("")); } @Test public void deleteMalformedGroupShouldReturnBadRequest() { - when() + Map<String, Object> errors = when() .delete("not-an-address" + SEPARATOR + USER_A) .then() - .statusCode(HttpStatus.BAD_REQUEST_400); + .statusCode(HttpStatus.BAD_REQUEST_400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "The group is not an email address") + .containsEntry("cause", "Out of data at position 1 in 'not-an-address'"); } @Test public void deleteMalformedAddressShouldReturnBadRequest() { - when() + Map<String, Object> errors = when() .delete(GROUP1 + SEPARATOR + "not-an-address") .then() - .statusCode(HttpStatus.BAD_REQUEST_400); + .statusCode(HttpStatus.BAD_REQUEST_400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "The group is not an email address") + .containsEntry("cause", "Out of data at position 1 in 'not-an-address'"); } @Test @@ -437,7 +526,8 @@ public class GroupsRoutesTest { when() .delete(GROUP1) .then() - .statusCode(HttpStatus.BAD_REQUEST_400); + .statusCode(HttpStatus.BAD_REQUEST_400) + .body(is("")); } @Test @@ -449,7 +539,8 @@ public class GroupsRoutesTest { when() .put(GROUP1 + SEPARATOR + GROUP2) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -461,7 +552,8 @@ public class GroupsRoutesTest { when() .put(GROUP1 + SEPARATOR + GROUP2) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -473,7 +565,8 @@ public class GroupsRoutesTest { when() .put(GROUP1 + SEPARATOR + GROUP2) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -485,7 +578,8 @@ public class GroupsRoutesTest { when() .get() .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -497,7 +591,8 @@ public class GroupsRoutesTest { when() .get() .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -509,7 +604,8 @@ public class GroupsRoutesTest { when() .get() .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -521,7 +617,8 @@ public class GroupsRoutesTest { when() .delete(GROUP1 + SEPARATOR + GROUP2) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -533,7 +630,8 @@ public class GroupsRoutesTest { when() .delete(GROUP1 + SEPARATOR + GROUP2) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -545,7 +643,8 @@ public class GroupsRoutesTest { when() .delete(GROUP1 + SEPARATOR + GROUP2) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -557,7 +656,8 @@ public class GroupsRoutesTest { when() .get(GROUP1) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -569,7 +669,8 @@ public class GroupsRoutesTest { when() .get(GROUP1) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } @Test @@ -581,7 +682,8 @@ public class GroupsRoutesTest { when() .get(GROUP1) .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); + .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500) + .body(containsString("500 Internal Server Error")); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/GlobalQuotaRoutes.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/GlobalQuotaRoutes.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/GlobalQuotaRoutes.java index a8f7e62..a45bff2 100644 --- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/GlobalQuotaRoutes.java +++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/GlobalQuotaRoutes.java @@ -19,6 +19,8 @@ package org.apache.james.webadmin.routes; +import static spark.Spark.halt; + import javax.inject.Inject; import javax.ws.rs.DELETE; import javax.ws.rs.GET; @@ -32,9 +34,14 @@ import org.apache.james.webadmin.Constants; import org.apache.james.webadmin.Routes; import org.apache.james.webadmin.dto.QuotaDTO; import org.apache.james.webadmin.dto.QuotaRequest; +import org.apache.james.webadmin.utils.ErrorResponder; +import org.apache.james.webadmin.utils.ErrorResponder.ErrorType; 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 org.slf4j.Logger; +import org.slf4j.LoggerFactory; import io.swagger.annotations.Api; import io.swagger.annotations.ApiImplicitParam; @@ -42,8 +49,6 @@ import io.swagger.annotations.ApiImplicitParams; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import spark.Service; @Api(tags = "GlobalQuota") @@ -108,10 +113,20 @@ public class GlobalQuotaRoutes implements Routes { response.status(204); } catch (JsonExtractException e) { LOGGER.info("Malformed JSON", e); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Malformed JSON input") + .cause(e) + .haltError(); } catch (IllegalArgumentException e) { LOGGER.info("Quota should be positive or unlimited (-1)", e); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Quota should be positive or unlimited (-1)") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; })); @@ -170,7 +185,12 @@ public class GlobalQuotaRoutes implements Routes { response.status(204); } catch (IllegalArgumentException e) { LOGGER.info("Invalid quota. Need to be an integer value greater than 0"); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid quota. Need to be an integer value greater than 0") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; }); @@ -225,7 +245,12 @@ public class GlobalQuotaRoutes implements Routes { response.status(204); } catch (IllegalArgumentException e) { LOGGER.info("Invalid quota. Need to be an integer value greater than 0"); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid quota. Need to be an integer value greater than 0") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; }); http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java index 1b218f5..2867c20 100644 --- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java +++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java @@ -29,9 +29,14 @@ import javax.ws.rs.Produces; import org.apache.james.webadmin.Constants; import org.apache.james.webadmin.Routes; import org.apache.james.webadmin.service.UserMailboxesService; +import org.apache.james.webadmin.utils.ErrorResponder; +import org.apache.james.webadmin.utils.ErrorResponder.ErrorType; import org.apache.james.webadmin.utils.JsonTransformer; import org.apache.james.webadmin.utils.MailboxHaveChildrenException; import org.apache.james.webadmin.validation.MailboxName; +import org.eclipse.jetty.http.HttpStatus; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import io.swagger.annotations.Api; import io.swagger.annotations.ApiImplicitParam; @@ -39,8 +44,6 @@ import io.swagger.annotations.ApiImplicitParams; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import spark.Service; @Api(tags = "User's Mailbox") @@ -100,8 +103,12 @@ public class UserMailboxesRoutes implements Routes { return userMailboxesService.listMailboxes(request.params(USER_NAME)); } catch (IllegalStateException e) { LOGGER.info("Invalid get on user mailboxes", e); - response.status(404); - return Constants.EMPTY_BODY; + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid get on user mailboxes") + .cause(e) + .haltError(); } }, jsonTransformer); } @@ -127,13 +134,28 @@ public class UserMailboxesRoutes implements Routes { response.status(204); } catch (IllegalStateException e) { LOGGER.info("Invalid delete on user mailbox", e); - response.status(404); + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid delete on user mailboxes") + .cause(e) + .haltError(); } catch (MailboxHaveChildrenException e) { LOGGER.info("Attempt to delete a mailbox with children"); - response.status(409); + throw ErrorResponder.builder() + .statusCode(HttpStatus.CONFLICT_409) + .type(ErrorType.INVALID_ARGUMENT) + .message("Attempt to delete a mailbox with children") + .cause(e) + .haltError(); } catch (IllegalArgumentException e) { LOGGER.info("Attempt to create an invalid mailbox"); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Attempt to create an invalid mailbox") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; }); @@ -157,7 +179,12 @@ public class UserMailboxesRoutes implements Routes { response.status(204); } catch (IllegalStateException e) { LOGGER.info("Invalid delete on user mailboxes", e); - response.status(404); + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid delete on user mailboxes") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; }); @@ -183,14 +210,28 @@ public class UserMailboxesRoutes implements Routes { if (userMailboxesService.testMailboxExists(request.params(USER_NAME), new MailboxName(request.params(MAILBOX_NAME)))) { response.status(204); } else { - response.status(404); + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid get on user mailboxes") + .haltError(); } } catch (IllegalStateException e) { LOGGER.info("Invalid get on user mailbox", e); - response.status(404); + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid get on user mailboxes") + .cause(e) + .haltError(); } catch (IllegalArgumentException e) { LOGGER.info("Attempt to create an invalid mailbox"); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Attempt to create an invalid mailbox") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; }); @@ -217,10 +258,20 @@ public class UserMailboxesRoutes implements Routes { response.status(204); } catch (IllegalStateException e) { LOGGER.info("Invalid put on user mailbox", e); - response.status(404); + throw ErrorResponder.builder() + .statusCode(HttpStatus.NOT_FOUND_404) + .type(ErrorType.INVALID_ARGUMENT) + .message("Invalid get on user mailboxes") + .cause(e) + .haltError(); } catch (IllegalArgumentException e) { LOGGER.info("Attempt to create an invalid mailbox"); - response.status(400); + throw ErrorResponder.builder() + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorType.INVALID_ARGUMENT) + .message("Attempt to create an invalid mailbox") + .cause(e) + .haltError(); } return Constants.EMPTY_BODY; }); http://git-wip-us.apache.org/repos/asf/james-project/blob/bde6bdb7/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/GlobalQuotaRoutesTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/GlobalQuotaRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/GlobalQuotaRoutesTest.java index a1e3971..5f79edf 100644 --- a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/GlobalQuotaRoutesTest.java +++ b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/GlobalQuotaRoutesTest.java @@ -24,6 +24,9 @@ import static com.jayway.restassured.config.EncoderConfig.encoderConfig; import static com.jayway.restassured.config.RestAssuredConfig.newConfig; import static org.apache.james.webadmin.WebAdminServer.NO_CONFIGURATION; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.CoreMatchers.is; + +import java.util.Map; import org.apache.james.mailbox.inmemory.quota.InMemoryPerUserMaxQuotaManager; import org.apache.james.mailbox.model.Quota; @@ -101,20 +104,41 @@ public class GlobalQuotaRoutesTest { @Test public void putCountShouldRejectInvalid() throws Exception { - given() + Map<String, Object> errors = given() .body("invalid") .put(GlobalQuotaRoutes.COUNT_ENDPOINT) .then() - .statusCode(400); + .statusCode(400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Invalid quota. Need to be an integer value greater than 0") + .containsEntry("cause", "For input string: \"invalid\""); } @Test public void putCountShouldRejectNegative() throws Exception { - given() + Map<String, Object> errors = given() .body("-1") .put(GlobalQuotaRoutes.COUNT_ENDPOINT) .then() - .statusCode(400); + .statusCode(400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Invalid quota. Need to be an integer value greater than 0"); } @Test @@ -174,20 +198,41 @@ public class GlobalQuotaRoutesTest { @Test public void putSizeShouldRejectInvalid() throws Exception { - given() + Map<String, Object> errors = given() .body("invalid") .put(GlobalQuotaRoutes.SIZE_ENDPOINT) .then() - .statusCode(400); + .statusCode(400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Invalid quota. Need to be an integer value greater than 0") + .containsEntry("cause", "For input string: \"invalid\""); } @Test public void putSizeShouldRejectNegative() throws Exception { - given() + Map<String, Object> errors = given() .body("-1") .put(GlobalQuotaRoutes.SIZE_ENDPOINT) .then() - .statusCode(400); + .statusCode(400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", 400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Invalid quota. Need to be an integer value greater than 0"); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
