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/11/19 02:43:38 UTC
[james-project] 36/43: JAMES-2982 UserRoutes + UserService improve
error messages and tests
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 b22ceab4e191359e94bafb3c11dc3767a64c3ee8
Author: Tran Tien Duc <dt...@linagora.com>
AuthorDate: Mon Nov 18 10:51:05 2019 +0700
JAMES-2982 UserRoutes + UserService improve error messages and tests
- Tests now includes nonVirtualHosting repository
- Rely some generic exception on common error handling
---
server/protocols/webadmin/webadmin-data/pom.xml | 6 +
.../apache/james/webadmin/routes/UserRoutes.java | 59 +-
.../apache/james/webadmin/service/UserService.java | 40 +-
.../james/webadmin/routes/UserRoutesTest.java | 620 +++++++++++++++++++++
.../james/webadmin/routes/UsersRoutesTest.java | 464 ---------------
5 files changed, 653 insertions(+), 536 deletions(-)
diff --git a/server/protocols/webadmin/webadmin-data/pom.xml b/server/protocols/webadmin/webadmin-data/pom.xml
index 8694906..fedd3c4 100644
--- a/server/protocols/webadmin/webadmin-data/pom.xml
+++ b/server/protocols/webadmin/webadmin-data/pom.xml
@@ -56,6 +56,12 @@
</dependency>
<dependency>
<groupId>${james.groupId}</groupId>
+ <artifactId>james-server-data-library</artifactId>
+ <scope>test</scope>
+ <type>test-jar</type>
+ </dependency>
+ <dependency>
+ <groupId>${james.groupId}</groupId>
<artifactId>james-server-data-memory</artifactId>
<scope>test</scope>
</dependency>
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 9006e96..6d3b641 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
@@ -20,6 +20,7 @@
package org.apache.james.webadmin.routes;
import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static spark.Spark.halt;
import javax.inject.Inject;
import javax.ws.rs.DELETE;
@@ -28,6 +29,8 @@ import javax.ws.rs.PUT;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
+import org.apache.james.core.Username;
+import org.apache.james.user.api.InvalidUsernameException;
import org.apache.james.user.api.UsersRepositoryException;
import org.apache.james.webadmin.Routes;
import org.apache.james.webadmin.dto.AddUserRequest;
@@ -49,6 +52,7 @@ 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.Service;
@@ -139,7 +143,7 @@ public class UserRoutes implements Routes {
}
private String removeUser(Request request, Response response) {
- String username = request.params(USER_NAME);
+ Username username = extractUsername(request);
try {
userService.removeUser(username);
return Responses.returnNoContent(response);
@@ -150,54 +154,37 @@ public class UserRoutes implements Routes {
.message("The user " + username + " does not exists")
.cause(e)
.haltError();
- } catch (UserService.InvalidUsername e) {
- LOGGER.info("Invalid username", e);
- throw ErrorResponder.builder()
- .statusCode(HttpStatus.BAD_REQUEST_400)
- .type(ErrorType.INVALID_ARGUMENT)
- .message("Invalid username: it should be between 1 and 255 long without '/'")
- .cause(e)
- .haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Invalid user path", e);
- throw ErrorResponder.builder()
- .statusCode(HttpStatus.BAD_REQUEST_400)
- .type(ErrorType.INVALID_ARGUMENT)
- .message("Invalid user path")
- .cause(e)
- .haltError();
}
}
- private String upsertUser(Request request, Response response) throws UsersRepositoryException {
+ private HaltException upsertUser(Request request, Response response) throws JsonExtractException {
+ Username username = extractUsername(request);
try {
- return userService.upsertUser(request.params(USER_NAME),
- jsonExtractor.parse(request.body()).getPassword(),
- response);
- } catch (JsonExtractException e) {
- LOGGER.info("Error while deserializing addUser request", e);
- throw ErrorResponder.builder()
- .statusCode(HttpStatus.BAD_REQUEST_400)
- .type(ErrorType.INVALID_ARGUMENT)
- .message("Error while deserializing addUser request")
- .cause(e)
- .haltError();
- } catch (UserService.InvalidUsername e) {
+ userService.upsertUser(username,
+ jsonExtractor.parse(request.body()).getPassword());
+
+ return halt(HttpStatus.NO_CONTENT_204);
+ } catch (InvalidUsernameException e) {
LOGGER.info("Invalid username", e);
throw ErrorResponder.builder()
.statusCode(HttpStatus.BAD_REQUEST_400)
.type(ErrorType.INVALID_ARGUMENT)
- .message("Invalid username: it should be between 1 and 255 long without '/'")
+ .message("Username supplied is invalid")
.cause(e)
.haltError();
- } catch (IllegalArgumentException e) {
- LOGGER.info("Invalid user path", e);
+ } catch (UsersRepositoryException e) {
+ String errorMessage = String.format("Error while upserting user '%s'", username);
+ LOGGER.info(errorMessage, e);
throw ErrorResponder.builder()
- .statusCode(HttpStatus.BAD_REQUEST_400)
- .type(ErrorType.INVALID_ARGUMENT)
- .message("Invalid user path")
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500)
+ .type(ErrorType.SERVER_ERROR)
+ .message(errorMessage)
.cause(e)
.haltError();
}
}
+
+ private Username extractUsername(Request request) {
+ return Username.of(request.params(USER_NAME));
+ }
}
\ No newline at end of file
diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
index b215803..78501da 100644
--- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
+++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
@@ -31,26 +31,11 @@ import org.apache.james.user.api.UsersRepositoryException;
import org.apache.james.user.api.model.User;
import org.apache.james.util.streams.Iterators;
import org.apache.james.webadmin.dto.UserResponse;
-import org.apache.james.webadmin.utils.Responses;
-import org.eclipse.jetty.http.HttpStatus;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import com.github.steveash.guavate.Guavate;
-import spark.Response;
-
public class UserService {
- public static class InvalidUsername extends RuntimeException {
- InvalidUsername(Exception e) {
- super("Username invariants violation", e);
- }
- }
-
- private static final Logger LOGGER = LoggerFactory.getLogger(UserService.class);
- private static final String EMPTY_BODY = "";
-
private final UsersRepository usersRepository;
@Inject
@@ -67,32 +52,15 @@ public class UserService {
.collect(Guavate.toImmutableList());
}
- public void removeUser(String username) throws UsersRepositoryException {
- usernamePreconditions(username);
- usersRepository.removeUser(Username.of(username));
+ public void removeUser(Username username) throws UsersRepositoryException {
+ usersRepository.removeUser(username);
}
- public String upsertUser(String rawUsername, char[] password, Response response) throws UsersRepositoryException {
- usernamePreconditions(rawUsername);
- Username username = Username.of(rawUsername);
+ public void upsertUser(Username username, char[] password) throws UsersRepositoryException {
User user = usersRepository.getUserByName(username);
- try {
- upsert(user, username, password);
- return Responses.returnNoContent(response);
- } catch (UsersRepositoryException e) {
- LOGGER.info("Error creating or updating user : {}", e.getMessage());
- response.status(HttpStatus.CONFLICT_409);
- }
- return EMPTY_BODY;
+ upsert(user, username, password);
}
- private void usernamePreconditions(String username) {
- try {
- Username.of(username);
- } catch (IllegalArgumentException e) {
- throw new InvalidUsername(e);
- }
- }
private void upsert(User user, Username username, char[] password) throws UsersRepositoryException {
if (user == null) {
diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java
new file mode 100644
index 0000000..34a59a2
--- /dev/null
+++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java
@@ -0,0 +1,620 @@
+/****************************************************************
+ * 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 static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static io.restassured.RestAssured.with;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.james.core.Domain;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.DomainList;
+import org.apache.james.domainlist.api.mock.SimpleDomainList;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.user.api.model.User;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.service.UserService;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import com.google.common.collect.ImmutableMap;
+
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+
+class UserRoutesTest {
+
+ private static class UserRoutesExtension implements BeforeEachCallback, AfterEachCallback, ParameterResolver {
+
+ static UserRoutesExtension withVirtualHosting() {
+ return new UserRoutesExtension(MemoryUsersRepository.withVirtualHosting());
+ }
+ static UserRoutesExtension withoutVirtualHosting() {
+ return new UserRoutesExtension(MemoryUsersRepository.withoutVirtualHosting());
+ }
+
+ final MemoryUsersRepository usersRepository;
+
+ WebAdminServer webAdminServer;
+
+ UserRoutesExtension(MemoryUsersRepository usersRepository) {
+ this.usersRepository = spy(usersRepository);
+ }
+
+ @Override
+ public void beforeEach(ExtensionContext extensionContext) throws Exception {
+ DomainList domainList = new SimpleDomainList();
+ domainList.addDomain(DOMAIN);
+ usersRepository.setDomainList(domainList);
+
+ webAdminServer = startServer(usersRepository);
+ }
+
+ @Override
+ public void afterEach(ExtensionContext extensionContext) throws Exception {
+ webAdminServer.destroy();
+ }
+
+ @Override
+ public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
+ return parameterContext.getParameter()
+ .getType()
+ .isAssignableFrom(UsersRepository.class);
+ }
+
+ @Override
+ public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
+ return usersRepository;
+ }
+
+ private WebAdminServer startServer(UsersRepository usersRepository) {
+ WebAdminServer server = WebAdminUtils.createWebAdminServer(new UserRoutes(new UserService(usersRepository), new JsonTransformer()))
+ .start();
+
+ RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(server)
+ .setBasePath(UserRoutes.USERS)
+ .build();
+
+ return server;
+ }
+ }
+
+ interface UserRoutesContract {
+ interface AllContracts extends NormalBehaviourContract, MockBehaviorErrorHandlingContract {
+ }
+
+ interface NormalBehaviourContract {
+
+ @Test
+ default void getUsersShouldBeEmptyByDefault() {
+ List<Map<String, String>> users =
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.OK_200)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getList(".");
+
+ assertThat(users).isEmpty();
+ }
+
+ @Test
+ default void putShouldReturnUserErrorWhenNoBody() {
+ when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("JSON payload of the request is not valid"));
+ }
+
+ @Test
+ default void postShouldReturnUserErrorWhenEmptyJsonBody() {
+ given()
+ .body("{}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("JSON payload of the request is not valid"));
+ }
+
+ @Test
+ default void putShouldReturnUserErrorWhenWrongJsonBody() {
+ given()
+ .body("{\"bad\":\"any\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("JSON payload of the request is not valid"));
+ }
+
+ @Test
+ default void putShouldReturnRequireNonNullPassword() {
+ given()
+ .body("{\"password\":null}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("JSON payload of the request is not valid"));
+ }
+
+ @Test
+ default void deleteShouldReturnOk() {
+ when()
+ .delete(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+ }
+
+ @Test
+ default void deleteShouldReturnBadRequestWhenEmptyUserName() {
+ when()
+ .delete("/")
+ .then()
+ .statusCode(HttpStatus.NOT_FOUND_404);
+ }
+
+ @Test
+ default void deleteShouldReturnBadRequestWhenUsernameIsTooLong() {
+ when()
+ .delete(USERNAME_WITH_DOMAIN.asString() + "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(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("Invalid arguments supplied in the user request"))
+ .body("details", is("username length should not be longer than 255 characters"));
+ }
+
+ @Test
+ default void deleteShouldReturnNotFoundWhenUsernameContainsSlash() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString() + "/" + USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NOT_FOUND_404);
+ }
+
+ @Test
+ default void putShouldReturnBadRequestWhenEmptyUserName() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put("/")
+ .then()
+ .statusCode(HttpStatus.NOT_FOUND_404);
+ }
+
+ @Test
+ default void putShouldReturnBadRequestWhenUsernameIsTooLong() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString() + "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(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("Invalid arguments supplied in the user request"))
+ .body("details", is("username length should not be longer than 255 characters"));
+ }
+
+ @Test
+ default void putShouldReturnNotFoundWhenUsernameContainsSlash() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString() + "/" + USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NOT_FOUND_404);
+ }
+
+ @Test
+ default void deleteShouldRemoveAssociatedUser() {
+ // Given
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(USERNAME_WITH_DOMAIN.asString());
+
+ // When
+ when()
+ .delete(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+
+ // Then
+ List<Map<String, String>> users =
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.OK_200)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getList(".");
+
+ assertThat(users).isEmpty();
+ }
+
+ @Test
+ default void deleteShouldStillBeValidWithExtraBody() {
+ given()
+ .body("{\"bad\":\"any\"}")
+ .when()
+ .delete(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+ }
+ }
+
+ interface MockBehaviorErrorHandlingContract {
+
+ @Test
+ default void deleteShouldStillBeOkWhenNoUser(UsersRepository usersRepository) throws Exception {
+ doThrow(new UsersRepositoryException("message")).when(usersRepository).removeUser(USERNAME_WITH_DOMAIN);
+
+ when()
+ .delete(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+ }
+
+ @Test
+ default void getShouldFailOnRepositoryException(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.list()).thenThrow(new UsersRepositoryException("message"));
+
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+ @Test
+ default void putShouldFailOnRepositoryExceptionOnGetUserByName(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenThrow(new UsersRepositoryException("message"));
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+ @Test
+ default void putShouldReturnInternalServerErrorWhenUserRepositoryAddingUserError(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenReturn(null);
+ doThrow(new UsersRepositoryException("message")).when(usersRepository).addUser(USERNAME_WITH_DOMAIN, PASSWORD);
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+ @Test
+ default void putShouldReturnInternalServerErrorWhenUserRepositoryUpdatingUserError(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenReturn(mock(User.class));
+ doThrow(new UsersRepositoryException("message")).when(usersRepository).updateUser(any());
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+
+ @Test
+ default void deleteShouldFailOnUnknownException(UsersRepository usersRepository) throws Exception {
+ doThrow(new RuntimeException()).when(usersRepository).removeUser(USERNAME_WITH_DOMAIN);
+
+ when()
+ .delete(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+ @Test
+ default void getShouldFailOnUnknownException(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.list()).thenThrow(new RuntimeException());
+
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+ @Test
+ default void putShouldFailOnUnknownExceptionOnGetUserByName(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenThrow(new RuntimeException());
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+ @Test
+ default void putShouldFailOnUnknownExceptionOnAddUser(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenReturn(null);
+ doThrow(new RuntimeException()).when(usersRepository).addUser(USERNAME_WITH_DOMAIN, PASSWORD);
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+
+ @Test
+ default void putShouldFailOnUnknownExceptionOnGetUpdateUser(UsersRepository usersRepository) throws Exception {
+ when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenReturn(mock(User.class));
+ doThrow(new RuntimeException()).when(usersRepository).updateUser(any());
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
+ }
+ }
+ }
+
+ private static final Domain DOMAIN = Domain.of("domain");
+ private static final Username USERNAME_WITHOUT_DOMAIN = Username.of("username");
+ private static final Username USERNAME_WITH_DOMAIN =
+ Username.fromLocalPartWithDomain(USERNAME_WITHOUT_DOMAIN.asString(), DOMAIN);
+ private static final String PASSWORD = "password";
+
+ @Nested
+ class WithVirtualHosting implements UserRoutesContract.AllContracts {
+
+ @RegisterExtension
+ UserRoutesExtension extension = UserRoutesExtension.withVirtualHosting();
+
+ @Test
+ void puttingWithDomainPartInUsernameTwoTimesShouldBeAllowed() {
+ // Given
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(USERNAME_WITH_DOMAIN.asString());
+
+ // When
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+
+ // Then
+ List<Map<String, String>> users =
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.OK_200)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getList(".");
+
+ assertThat(users).containsExactly(ImmutableMap.of("username", USERNAME_WITH_DOMAIN.asString()));
+ }
+
+ @Test
+ void putWithDomainPartInUsernameShouldReturnOkWhenWithA255LongUsername() {
+ String usernameTail = "@" + DOMAIN.name();
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(StringUtils.repeat('j', 255 - usernameTail.length()) + usernameTail)
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+ }
+
+ @Test
+ void putWithDomainPartInUsernameShouldAddTheUser() {
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(USERNAME_WITH_DOMAIN.asString());
+
+ List<Map<String, String>> users =
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.OK_200)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getList(".");
+
+ assertThat(users).containsExactly(ImmutableMap.of("username", USERNAME_WITH_DOMAIN.asString()));
+ }
+
+ @Test
+ void putShouldReturnBadRequestWhenUsernameDoesNotHaveDomainPart() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put("justLocalPart")
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("Username supplied is invalid"))
+ .body("details", is("Given Username needs to contain a @domainpart"));
+ }
+
+ @Test
+ void putWithDomainPartInUsernameShouldReturnOkWhenValidJsonBody() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+ }
+ }
+
+ @Nested
+ class WithoutVirtualHosting implements UserRoutesContract.AllContracts {
+
+ @RegisterExtension
+ UserRoutesExtension extension = UserRoutesExtension.withoutVirtualHosting();
+
+ @Test
+ void puttingWithoutDomainPartInUsernameTwoTimesShouldBeAllowed() {
+ // Given
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(USERNAME_WITHOUT_DOMAIN.asString());
+
+ // When
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITHOUT_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+
+ // Then
+ List<Map<String, String>> users =
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.OK_200)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getList(".");
+
+ assertThat(users).containsExactly(ImmutableMap.of("username", USERNAME_WITHOUT_DOMAIN.asString()));
+ }
+
+ @Test
+ void putWithoutDomainPartInUsernameShouldReturnOkWhenWithA255LongUsername() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(StringUtils.repeat('j',
+ 255 - USERNAME_WITHOUT_DOMAIN.asString().length()) + USERNAME_WITHOUT_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+ }
+
+ @Test
+ void putWithoutDomainPartInUsernameShouldAddTheUser() {
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(USERNAME_WITHOUT_DOMAIN.asString());
+
+ List<Map<String, String>> users =
+ when()
+ .get()
+ .then()
+ .statusCode(HttpStatus.OK_200)
+ .contentType(ContentType.JSON)
+ .extract()
+ .body()
+ .jsonPath()
+ .getList(".");
+
+ assertThat(users).containsExactly(ImmutableMap.of("username", USERNAME_WITHOUT_DOMAIN.asString()));
+ }
+
+ @Test
+ void putShouldReturnBadRequestWhenUsernameHasDomainPart() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.BAD_REQUEST_400)
+ .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+ .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+ .body("message", is("Username supplied is invalid"))
+ .body("details", is("Given Username contains a @domainpart but virtualhosting support is disabled"));
+ }
+
+ @Test
+ void putWithoutDomainPartInUsernameShouldReturnOkWhenValidJsonBody() {
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITHOUT_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
+ }
+ }
+}
diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UsersRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UsersRoutesTest.java
deleted file mode 100644
index a2fe544..0000000
--- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UsersRoutesTest.java
+++ /dev/null
@@ -1,464 +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 static io.restassured.RestAssured.given;
-import static io.restassured.RestAssured.when;
-import static io.restassured.RestAssured.with;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.hamcrest.Matchers.is;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import java.util.List;
-import java.util.Map;
-
-import org.apache.commons.lang3.StringUtils;
-import org.apache.james.core.Domain;
-import org.apache.james.core.Username;
-import org.apache.james.domainlist.api.DomainList;
-import org.apache.james.user.api.UsersRepository;
-import org.apache.james.user.api.UsersRepositoryException;
-import org.apache.james.user.api.model.User;
-import org.apache.james.user.memory.MemoryUsersRepository;
-import org.apache.james.webadmin.WebAdminServer;
-import org.apache.james.webadmin.WebAdminUtils;
-import org.apache.james.webadmin.service.UserService;
-import org.apache.james.webadmin.utils.ErrorResponder;
-import org.apache.james.webadmin.utils.JsonTransformer;
-import org.eclipse.jetty.http.HttpStatus;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Nested;
-import org.junit.jupiter.api.Test;
-
-import com.google.common.collect.ImmutableMap;
-
-import io.restassured.RestAssured;
-import io.restassured.http.ContentType;
-
-class UsersRoutesTest {
-
- private static final Domain DOMAIN = Domain.of("domain");
- private static final String USERNAME = "username@" + DOMAIN.name();
- private WebAdminServer webAdminServer;
-
- private void createServer(UsersRepository usersRepository) {
- webAdminServer = WebAdminUtils.createWebAdminServer(new UserRoutes(new UserService(usersRepository), new JsonTransformer()))
- .start();
-
- RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer)
- .setBasePath(UserRoutes.USERS)
- .build();
- }
-
- @AfterEach
- void stop() {
- webAdminServer.destroy();
- }
-
- @Nested
- class NormalBehaviour {
-
- @BeforeEach
- void setUp() throws Exception {
- DomainList domainList = mock(DomainList.class);
- when(domainList.containsDomain(DOMAIN)).thenReturn(true);
-
- MemoryUsersRepository usersRepository = MemoryUsersRepository.withVirtualHosting();
- usersRepository.setDomainList(domainList);
-
- createServer(usersRepository);
- }
-
- @Test
- void getUsersShouldBeEmptyByDefault() {
- List<Map<String, String>> users =
- when()
- .get()
- .then()
- .statusCode(HttpStatus.OK_200)
- .contentType(ContentType.JSON)
- .extract()
- .body()
- .jsonPath()
- .getList(".");
-
- assertThat(users).isEmpty();
- }
-
- @Test
- void putShouldReturnUserErrorWhenNoBody() {
- when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.BAD_REQUEST_400);
- }
-
- @Test
- void postShouldReturnUserErrorWhenEmptyJsonBody() {
- given()
- .body("{}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.BAD_REQUEST_400);
- }
-
- @Test
- void putShouldReturnUserErrorWhenWrongJsonBody() {
- given()
- .body("{\"bad\":\"any\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.BAD_REQUEST_400);
- }
-
- @Test
- void putShouldReturnOkWhenValidJsonBody() {
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.NO_CONTENT_204);
- }
-
- @Test
- void putShouldReturnOkWhenWithA255LongUsername() {
- String usernameTail = "@" + DOMAIN.name();
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(StringUtils.repeat('j', 255 - usernameTail.length()) + usernameTail)
- .then()
- .statusCode(HttpStatus.NO_CONTENT_204);
- }
-
- @Test
- void putShouldReturnRequireNonNullPassword() {
- given()
- .body("{\"password\":null}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.BAD_REQUEST_400);
- }
-
- @Test
- void putShouldAddTheUser() {
- with()
- .body("{\"password\":\"password\"}")
- .put(USERNAME);
-
- List<Map<String, String>> users =
- when()
- .get()
- .then()
- .statusCode(HttpStatus.OK_200)
- .contentType(ContentType.JSON)
- .extract()
- .body()
- .jsonPath()
- .getList(".");
-
- assertThat(users).containsExactly(ImmutableMap.of("username", USERNAME));
- }
-
- @Test
- void puttingTwoTimesShouldBeAllowed() {
- // Given
- with()
- .body("{\"password\":\"password\"}")
- .put(USERNAME);
-
- // When
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.NO_CONTENT_204);
-
- // Then
- List<Map<String, String>> users =
- when()
- .get()
- .then()
- .statusCode(HttpStatus.OK_200)
- .contentType(ContentType.JSON)
- .extract()
- .body()
- .jsonPath()
- .getList(".");
-
- assertThat(users).containsExactly(ImmutableMap.of("username", USERNAME));
- }
-
- @Test
- void deleteShouldReturnOk() {
- when()
- .delete(USERNAME)
- .then()
- .statusCode(HttpStatus.NO_CONTENT_204);
- }
-
- @Test
- void deleteShouldReturnBadRequestWhenEmptyUserName() {
- when()
- .delete("/")
- .then()
- .statusCode(HttpStatus.NOT_FOUND_404);
- }
-
- @Test
- void deleteShouldReturnBadRequestWhenUsernameIsTooLong() {
- when()
- .delete(USERNAME + "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(HttpStatus.BAD_REQUEST_400)
- .body("statusCode", is(400))
- .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
- .body("message", is("Invalid username: it should be between 1 and 255 long without '/'"));
- }
-
- @Test
- void deleteShouldReturnNotFoundWhenUsernameContainsSlash() {
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME + "/" + USERNAME)
- .then()
- .statusCode(HttpStatus.NOT_FOUND_404);
- }
-
- @Test
- void putShouldReturnBadRequestWhenEmptyUserName() {
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put("/")
- .then()
- .statusCode(HttpStatus.NOT_FOUND_404);
- }
-
- @Test
- void putShouldReturnBadRequestWhenUsernameIsTooLong() {
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME + "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(HttpStatus.BAD_REQUEST_400)
- .body("statusCode", is(400))
- .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
- .body("message", is("Invalid username: it should be between 1 and 255 long without '/'"));
- }
-
- @Test
- void putShouldReturnNotFoundWhenUsernameContainsSlash() {
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME + "/" + USERNAME)
- .then()
- .statusCode(HttpStatus.NOT_FOUND_404);
- }
-
- @Test
- void deleteShouldRemoveAssociatedUser() {
- // Given
- with()
- .body("{\"password\":\"password\"}")
- .put(USERNAME);
-
- // When
- when()
- .delete(USERNAME)
- .then()
- .statusCode(HttpStatus.NO_CONTENT_204);
-
- // Then
- List<Map<String, String>> users =
- when()
- .get()
- .then()
- .statusCode(HttpStatus.OK_200)
- .contentType(ContentType.JSON)
- .extract()
- .body()
- .jsonPath()
- .getList(".");
-
- assertThat(users).isEmpty();
- }
-
- @Test
- void deleteShouldStillBeValidWithExtraBody() {
- given()
- .body("{\"bad\":\"any\"}")
- .when()
- .delete(USERNAME)
- .then()
- .statusCode(HttpStatus.NO_CONTENT_204);
- }
- }
-
- @Nested
- class ErrorHandling {
-
- private UsersRepository usersRepository;
- private Username username;
- private String password;
-
- @BeforeEach
- void setUp() throws Exception {
- usersRepository = mock(UsersRepository.class);
- createServer(usersRepository);
- username = Username.of("username@domain");
- password = "password";
- }
-
- @Test
- void deleteShouldStillBeOkWhenNoUser() throws Exception {
- doThrow(new UsersRepositoryException("message")).when(usersRepository).removeUser(username);
-
- when()
- .delete(USERNAME)
- .then()
- .statusCode(HttpStatus.NO_CONTENT_204);
- }
-
- @Test
- void getShouldFailOnRepositoryException() throws Exception {
- when(usersRepository.list()).thenThrow(new UsersRepositoryException("message"));
-
- when()
- .get()
- .then()
- .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
- }
-
- @Test
- void putShouldFailOnRepositoryExceptionOnGetUserByName() throws Exception {
- when(usersRepository.getUserByName(username)).thenThrow(new UsersRepositoryException("message"));
-
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
- }
-
- @Test
- void putShouldNotFailOnRepositoryExceptionOnAddUser() throws Exception {
- when(usersRepository.getUserByName(username)).thenReturn(null);
- doThrow(new UsersRepositoryException("message")).when(usersRepository).addUser(username, password);
-
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.CONFLICT_409);
- }
-
- @Test
- void putShouldFailOnRepositoryExceptionOnUpdateUser() throws Exception {
- when(usersRepository.getUserByName(username)).thenReturn(mock(User.class));
- doThrow(new UsersRepositoryException("message")).when(usersRepository).updateUser(any());
-
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.CONFLICT_409);
- }
-
-
- @Test
- void deleteShouldFailOnUnknownException() throws Exception {
- doThrow(new RuntimeException()).when(usersRepository).removeUser(username);
-
- when()
- .delete(USERNAME)
- .then()
- .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
- }
-
- @Test
- void getShouldFailOnUnknownException() throws Exception {
- when(usersRepository.list()).thenThrow(new RuntimeException());
-
- when()
- .get()
- .then()
- .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
- }
-
- @Test
- void putShouldFailOnUnknownExceptionOnGetUserByName() throws Exception {
- when(usersRepository.getUserByName(username)).thenThrow(new RuntimeException());
-
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
- }
-
- @Test
- void putShouldFailOnUnknownExceptionOnAddUser() throws Exception {
- when(usersRepository.getUserByName(username)).thenReturn(null);
- doThrow(new RuntimeException()).when(usersRepository).addUser(username, password);
-
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
- }
-
- @Test
- void putShouldFailOnUnknownExceptionOnGetUpdateUser() throws Exception {
- when(usersRepository.getUserByName(username)).thenReturn(mock(User.class));
- doThrow(new RuntimeException()).when(usersRepository).updateUser(any());
-
- given()
- .body("{\"password\":\"password\"}")
- .when()
- .put(USERNAME)
- .then()
- .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500);
- }
- }
-
-}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org