From ce8e498b98fcd56eca2492a0b2e04b8686e0ace3 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 17 Dec 2014 12:37:02 +0100 Subject: [PATCH] SONAR-5934 User Creation WS in Java --- .../org/sonar/server/user/UserCreator.java | 52 ++++---- .../org/sonar/server/user/index/UserDoc.java | 3 +- .../sonar/server/user/ws/CreateAction.java | 69 ++++++---- .../org/sonar/server/user/ws/UsersWs.java | 38 +----- .../sonar/server/user/UserCreatorTest.java | 14 +++ .../server/user/ws/CreateActionTest.java | 119 ++++++++++++++++++ .../org/sonar/server/user/ws/UsersWsTest.java | 2 +- .../user/ws/CreateActionTest/create_user.json | 9 ++ .../return_409_when_reactive_exception.json | 9 ++ .../java/org/sonar/core/user/UserDto.java | 3 +- 10 files changed, 233 insertions(+), 85 deletions(-) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/create_user.json create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/return_409_when_reactive_exception.json diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java index fa185658951..0b4438a6713 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java @@ -40,6 +40,7 @@ import org.sonar.server.exceptions.Message; import org.sonar.server.user.db.UserGroupDao; import org.sonar.server.util.Validation; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import java.io.StringWriter; @@ -51,11 +52,11 @@ import static com.google.common.collect.Lists.newArrayList; public class UserCreator { - private static final String LOGIN = "Login"; - private static final String PASSWORD_CONFIRMATION = "Password confirmation"; - private static final String PASSWORD = "Password"; - private static final String NAME = "Name"; - private static final String EMAIL = "Email"; + private static final String LOGIN_PARAM = "Login"; + private static final String PASSWORD_CONFIRMATION_PARAM = "Password confirmation"; + private static final String PASSWORD_PARAM = "Password"; + private static final String NAME_PARAM = "Name"; + private static final String EMAIL_PARAM = "Email"; private final NewUserNotifier newUserNotifier; private final Settings settings; @@ -95,9 +96,7 @@ public class UserCreator { validateLogin(newUser.login(), messages); validateName(newUser.name(), messages); - if (newUser.email().length() >= 100) { - messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, EMAIL, 100)); - } + validateEmail(newUser.email(), messages); validatePassword(newUser, messages); if (!messages.isEmpty()) { @@ -107,30 +106,36 @@ public class UserCreator { private static void validateLogin(@Nullable String login, List messages) { if (Strings.isNullOrEmpty(login)) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, LOGIN)); + messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, LOGIN_PARAM)); } else if (!login.matches("\\A\\w[\\w\\.\\-_@\\s]+\\z")) { messages.add(Message.of("user.bad_login")); } else if (login.length() <= 2) { - messages.add(Message.of(Validation.IS_TOO_SHORT_MESSAGE, LOGIN, 2)); + messages.add(Message.of(Validation.IS_TOO_SHORT_MESSAGE, LOGIN_PARAM, 2)); } else if (login.length() >= 255) { - messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, LOGIN, 255)); + messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, LOGIN_PARAM, 255)); } } private static void validateName(@Nullable String name, List messages) { if (Strings.isNullOrEmpty(name)) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, NAME)); + messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, NAME_PARAM)); } else if (name.length() >= 200) { - messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, NAME, 200)); + messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, NAME_PARAM, 200)); + } + } + + private static void validateEmail(@Nullable String email, List messages) { + if (!Strings.isNullOrEmpty(email) && email.length() >= 100) { + messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, EMAIL_PARAM, 100)); } } private static void validatePassword(NewUser newUser, List messages) { if (Strings.isNullOrEmpty(newUser.password())) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, PASSWORD)); + messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, PASSWORD_PARAM)); } if (Strings.isNullOrEmpty(newUser.passwordConfirmation())) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, PASSWORD_CONFIRMATION)); + messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, PASSWORD_CONFIRMATION_PARAM)); } if (!Strings.isNullOrEmpty(newUser.password()) && !Strings.isNullOrEmpty(newUser.passwordConfirmation()) @@ -182,13 +187,18 @@ public class UserCreator { userDto.setCryptedPassword(DigestUtils.sha1Hex("--" + saltHex + "--" + newUser.password() + "--")); } + @CheckForNull private static String convertScmAccountsToCsv(NewUser newUser) { - int size = newUser.scmAccounts().size(); - StringWriter writer = new StringWriter(size); - CsvWriter csv = CsvWriter.of(writer); - csv.values(newUser.scmAccounts().toArray(new String[size])); - csv.close(); - return writer.toString(); + List scmAccounts = newUser.scmAccounts(); + if (scmAccounts != null) { + int size = newUser.scmAccounts().size(); + StringWriter writer = new StringWriter(size); + CsvWriter csv = CsvWriter.of(writer); + csv.values(newUser.scmAccounts().toArray(new String[size])); + csv.close(); + return writer.toString(); + } + return null; } private void notifyNewUser(NewUser newUser) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserDoc.java b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserDoc.java index 22ca523728f..d74f28bf1f8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserDoc.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserDoc.java @@ -49,9 +49,8 @@ public class UserDoc extends BaseDoc { return (Boolean) getField(UserIndexDefinition.FIELD_ACTIVE); } - @Nullable public List scmAccounts() { - return (List) getNullableField(UserIndexDefinition.FIELD_SCM_ACCOUNTS); + return (List) getField(UserIndexDefinition.FIELD_SCM_ACCOUNTS); } public long createdAt() { diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java index aa0bd94ff0b..cb5ea34f576 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java @@ -24,9 +24,14 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.RequestHandler; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; +import org.sonar.api.utils.text.JsonWriter; +import org.sonar.server.plugins.MimeTypes; import org.sonar.server.user.NewUser; import org.sonar.server.user.ReactivationException; import org.sonar.server.user.UserService; +import org.sonar.server.user.index.UserDoc; + +import java.io.OutputStreamWriter; public class CreateAction implements RequestHandler { @@ -35,12 +40,13 @@ public class CreateAction implements RequestHandler { private static final String PARAM_PASSWORD_CONFIRMATION = "password_confirmation"; private static final String PARAM_NAME = "name"; private static final String PARAM_EMAIL = "email"; + private static final String PARAM_SCM_ACCOUNTS = "scm_accounts"; private static final String PARAM_PREVENT_REACTIVATION = "prevent_reactivation"; - private final UserService userService; + private final UserService service; - public CreateAction(UserService userService) { - this.userService = userService; + public CreateAction(UserService service) { + this.service = service; } void define(WebService.NewController controller) { @@ -74,6 +80,10 @@ public class CreateAction implements RequestHandler { .setDescription("User email") .setExampleValue("myname@email.com"); + action.createParam(PARAM_SCM_ACCOUNTS) + .setDescription("SCM accounts") + .setExampleValue("myscmaccount1, myscmaccount2"); + action.createParam(PARAM_PREVENT_REACTIVATION) .setDescription("If set to true and if the user has been removed, a status 409 will be returned") .setDefaultValue(false) @@ -82,36 +92,49 @@ public class CreateAction implements RequestHandler { @Override public void handle(Request request, Response response) throws Exception { + String login = request.mandatoryParam(PARAM_LOGIN); NewUser newUser = NewUser.create() - .setLogin(request.mandatoryParam(PARAM_LOGIN)) + .setLogin(login) .setName(request.mandatoryParam(PARAM_NAME)) .setEmail(request.param(PARAM_EMAIL)) + .setScmAccounts(request.paramAsStrings(PARAM_SCM_ACCOUNTS)) .setPassword(request.mandatoryParam(PARAM_PASSWORD)) .setPasswordConfirmation(request.mandatoryParam(PARAM_PASSWORD_CONFIRMATION)) .setPreventReactivation(request.mandatoryParamAsBoolean(PARAM_PREVENT_REACTIVATION)); try { - userService.create(newUser); + service.create(newUser); + writeResponse(response, login); } catch (ReactivationException e) { - // write409(response, e.ruleKey()); + write409(response, login); } } - // private void writeResponse(Response response, RuleKey ruleKey) { - // Rule rule = service.getNonNullByKey(ruleKey); - // JsonWriter json = response.newJsonWriter().beginObject().name("rule"); - // mapping.write(rule, json, null /* TODO replace by SearchOptions immutable constant */); - // json.endObject().close(); - // } - // - // private void write409(Response response, RuleKey ruleKey) { - // Rule rule = service.getNonNullByKey(ruleKey); - // - // Response.Stream stream = response.stream(); - // stream.setStatus(409); - // stream.setMediaType(MimeTypes.JSON); - // JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output())).beginObject().name("rule"); - // mapping.write(rule, json, null /* TODO replace by SearchOptions immutable constant */); - // json.endObject().close(); - // } + private void writeResponse(Response response, String login) { + UserDoc user = service.getByLogin(login); + JsonWriter json = response.newJsonWriter().beginObject().name("user"); + writeUser(json, user); + json.endObject().close(); + } + + private void write409(Response response, String login) { + UserDoc user = service.getByLogin(login); + + Response.Stream stream = response.stream(); + stream.setStatus(409); + stream.setMediaType(MimeTypes.JSON); + JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output())).beginObject().name("user"); + writeUser(json, user); + json.endObject().close(); + } + + private void writeUser(JsonWriter json, UserDoc user) { + json.beginObject() + .prop("login", user.login()) + .prop("name", user.name()) + .prop("email", user.email()) + .prop("active", user.active()) + .name("scmAccounts").beginArray().values(user.scmAccounts()).endArray() + .endObject(); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java index 7fa5d64de63..a5b7d1abdf2 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java @@ -39,8 +39,7 @@ public class UsersWs implements WebService { .setDescription("Users management"); defineSearchAction(controller); -// createAction.define(controller); - defineCreateAction(controller); + createAction.define(controller); defineUpdateAction(controller); defineDeactivateAction(controller); @@ -66,41 +65,6 @@ public class UsersWs implements WebService { RailsHandler.addFormatParam(action); } - private void defineCreateAction(NewController controller) { - NewAction action = controller.createAction("create") - .setDescription("Create a user. Requires Administer System permission") - .setSince("3.7") - .setPost(true) - .setHandler(RailsHandler.INSTANCE); - - action.createParam("login") - .setDescription("User login") - .setRequired(true) - .setExampleValue("myuser"); - - action.createParam("password") - .setDescription("User password") - .setRequired(true) - .setExampleValue("mypassword"); - - action.createParam("password_confirmation") - .setDescription("Must be the same value as \"password\"") - .setRequired(true) - .setExampleValue("mypassword"); - - action.createParam("name") - .setDescription("User name") - .setRequired(true) - .setExampleValue("My Name"); - - action.createParam("email") - .setDescription("User email") - .setExampleValue("myname@email.com"); - - RailsHandler.addFormatParam(action); - } - - private void defineUpdateAction(NewController controller) { NewAction action = controller.createAction("update") .setDescription("Update a user. Requires Administer System permission") diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserCreatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserCreatorTest.java index 2b7ab1a6d4d..e76ca213b37 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/UserCreatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserCreatorTest.java @@ -125,6 +125,20 @@ public class UserCreatorTest { assertThat(dto.getUpdatedAt()).isEqualTo(1418215735482L); } + @Test + public void create_user_with_minimum_fields() throws Exception { + when(system2.now()).thenReturn(1418215735482L); + createDefaultGroup(); + + userCreator.create(NewUser.create() + .setLogin("user") + .setName("User") + .setPassword("password") + .setPasswordConfirmation("password")); + + assertThat(userDao.selectNullableByLogin(session, "user")).isNotNull(); + } + @Test public void fail_to_create_user_if_login_already_exists() throws Exception { db.prepareDbUnit(getClass(), "fail_to_create_user_if_already_exists.xml"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java new file mode 100644 index 00000000000..ae92a794896 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java @@ -0,0 +1,119 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.user.ws; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.sonar.api.server.ws.WebService; +import org.sonar.server.user.NewUser; +import org.sonar.server.user.ReactivationException; +import org.sonar.server.user.UserService; +import org.sonar.server.user.index.UserDoc; +import org.sonar.server.ws.WsTester; + +import java.util.Map; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.*; + +@RunWith(MockitoJUnitRunner.class) +public class CreateActionTest { + + WebService.Controller controller; + + WsTester tester; + + @Mock + UserService service; + + @Captor + ArgumentCaptor newUserCaptor; + + @Before + public void setUp() throws Exception { + tester = new WsTester(new UsersWs(new CreateAction(service))); + controller = tester.controller("api/users"); + } + + @Test + public void create_user() throws Exception { + Map userDocMap = newHashMap(); + userDocMap.put("login", "john"); + userDocMap.put("name", "John"); + userDocMap.put("email", "john@email.com"); + userDocMap.put("scmAccounts", newArrayList("jn")); + userDocMap.put("active", true); + userDocMap.put("createdAt", 15000L); + userDocMap.put("updatedAt", 15000L); + when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + + tester.newPostRequest("api/users", "create") + .setParam("login", "john") + .setParam("name", "John") + .setParam("email", "john@email.com") + .setParam("scm_accounts", "jn") + .setParam("password", "1234") + .setParam("password_confirmation", "1234").execute() + .assertJson(getClass(), "create_user.json"); + + verify(service).create(newUserCaptor.capture()); + assertThat(newUserCaptor.getValue().login()).isEqualTo("john"); + assertThat(newUserCaptor.getValue().name()).isEqualTo("John"); + assertThat(newUserCaptor.getValue().email()).isEqualTo("john@email.com"); + assertThat(newUserCaptor.getValue().scmAccounts()).containsOnly("jn"); + assertThat(newUserCaptor.getValue().password()).isEqualTo("1234"); + assertThat(newUserCaptor.getValue().passwordConfirmation()).isEqualTo("1234"); + assertThat(newUserCaptor.getValue().isPreventReactivation()).isFalse(); + } + + @Test + public void return_409_when_reactive_exception() throws Exception { + doThrow(new ReactivationException("Already exists", "john")).when(service).create(any(NewUser.class)); + + Map userDocMap = newHashMap(); + userDocMap.put("login", "john"); + userDocMap.put("name", "John"); + userDocMap.put("email", "john@email.com"); + userDocMap.put("scmAccounts", newArrayList("jn")); + userDocMap.put("active", true); + userDocMap.put("createdAt", 15000L); + userDocMap.put("updatedAt", 15000L); + when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + + tester.newPostRequest("api/users", "create") + .setParam("login", "john") + .setParam("name", "John2") + .setParam("email", "john2@email.com") + .setParam("scm_accounts", "jn2") + .setParam("password", "12345") + .setParam("password_confirmation", "12345").execute() + .assertStatus(409) + .assertJson(getClass(), "return_409_when_reactive_exception.json"); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java index 55bed235cd6..70ff4320a4e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java @@ -63,7 +63,7 @@ public class UsersWsTest { WebService.Action action = controller.action("create"); assertThat(action).isNotNull(); assertThat(action.isPost()).isTrue(); - assertThat(action.params()).hasSize(6); + assertThat(action.params()).hasSize(7); } @Test diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/create_user.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/create_user.json new file mode 100644 index 00000000000..0feda7c9c1c --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/create_user.json @@ -0,0 +1,9 @@ +{ + "user": { + "login": "john", + "name": "John", + "email": "john@email.com", + "scmAccounts": ["jn"], + "active": true + } +} diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/return_409_when_reactive_exception.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/return_409_when_reactive_exception.json new file mode 100644 index 00000000000..0feda7c9c1c --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/return_409_when_reactive_exception.json @@ -0,0 +1,9 @@ +{ + "user": { + "login": "john", + "name": "John", + "email": "john@email.com", + "scmAccounts": ["jn"], + "active": true + } +} diff --git a/sonar-core/src/main/java/org/sonar/core/user/UserDto.java b/sonar-core/src/main/java/org/sonar/core/user/UserDto.java index 14a174a8d98..3f62cf2fdbe 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/UserDto.java +++ b/sonar-core/src/main/java/org/sonar/core/user/UserDto.java @@ -83,11 +83,12 @@ public class UserDto { return this; } + @CheckForNull public String getScmAccounts() { return scmAccounts; } - public UserDto setScmAccounts(String scmAccounts) { + public UserDto setScmAccounts(@Nullable String scmAccounts) { this.scmAccounts = scmAccounts; return this; } -- 2.39.5