From 05f2a1332ceae679ae8c0e13cb95bb1eb4eccd29 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 10 Apr 2018 12:23:23 +0200 Subject: [PATCH] SONAR-10566 Allow organization name with one character --- .../organization/OrganizationValidation.java | 4 +- .../OrganizationValidationImpl.java | 4 +- .../server/organization/ws/CreateAction.java | 4 +- .../OrganizationValidationImplTest.java | 20 +- .../organization/ws/CreateActionTest.java | 520 +++++++++--------- .../organization/ws/UpdateActionTest.java | 6 +- .../tests/organization/OrganizationTest.java | 28 +- .../PersonalOrganizationTest.java | 16 +- 8 files changed, 298 insertions(+), 304 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidation.java b/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidation.java index 67c2ed3d547..ef41e6e3c80 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidation.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidation.java @@ -23,9 +23,9 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; public interface OrganizationValidation { - int KEY_MIN_LENGTH = 2; + int KEY_MIN_LENGTH = 1; int KEY_MAX_LENGTH = 32; - int NAME_MIN_LENGTH = 2; + int NAME_MIN_LENGTH = 1; int NAME_MAX_LENGTH = 64; int DESCRIPTION_MAX_LENGTH = 256; int URL_MAX_LENGTH = 256; diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidationImpl.java b/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidationImpl.java index 762bab7f838..516dfae762b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidationImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationValidationImpl.java @@ -32,7 +32,7 @@ public class OrganizationValidationImpl implements OrganizationValidation { @Override public String checkKey(String keyCandidate) { requireNonNull(keyCandidate, "key can't be null"); - checkArgument(keyCandidate.length() >= KEY_MIN_LENGTH, "Key '%s' must be at least %s chars long", keyCandidate, KEY_MIN_LENGTH); + checkArgument(keyCandidate.length() >= KEY_MIN_LENGTH, "Key must not be empty"); checkArgument(keyCandidate.length() <= KEY_MAX_LENGTH, "Key '%s' must be at most %s chars long", keyCandidate, KEY_MAX_LENGTH); checkArgument(slugify(keyCandidate).equals(keyCandidate), "Key '%s' contains at least one invalid char", keyCandidate); @@ -43,7 +43,7 @@ public class OrganizationValidationImpl implements OrganizationValidation { public String checkName(String nameCandidate) { requireNonNull(nameCandidate, "name can't be null"); - checkArgument(nameCandidate.length() >= NAME_MIN_LENGTH, "Name '%s' must be at least %s chars long", nameCandidate, NAME_MIN_LENGTH); + checkArgument(nameCandidate.length() >= NAME_MIN_LENGTH, "Name must not be empty"); checkArgument(nameCandidate.length() <= NAME_MAX_LENGTH, "Name '%s' must be at most %s chars long", nameCandidate, NAME_MAX_LENGTH); return nameCandidate; diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java index 601f413c27e..67815480444 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java @@ -22,6 +22,7 @@ package org.sonar.server.organization.ws; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.config.Configuration; +import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; @@ -73,6 +74,7 @@ public class CreateAction implements OrganizationsWsAction { .setResponseExample(getClass().getResource("create-example.json")) .setInternal(true) .setSince("6.2") + .setChangelog(new Change("7.2", "Minimal number of character of name and key is one character")) .setHandler(this); action.createParam(PARAM_KEY) @@ -81,7 +83,7 @@ public class CreateAction implements OrganizationsWsAction { .setDescription("Key of the organization.
" + "The key is unique to the whole SonarQube.
" + "When not specified, the key is computed from the name.
" + - "Otherwise, it must be between 2 and 32 chars long. All chars must be lower-case letters (a to z), digits or dash (but dash can neither be trailing nor heading)") + "Otherwise, it must be between 1 and 32 chars long. All chars must be lower-case letters (a to z), digits or dash (but dash can neither be trailing nor heading)") .setExampleValue("foo-company"); wsSupport.addOrganizationDetailsParams(action, true); diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationValidationImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationValidationImplTest.java index ec7b52713b9..edd43f5fe01 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationValidationImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationValidationImplTest.java @@ -48,17 +48,17 @@ public class OrganizationValidationImplTest { @Test public void checkValidKey_throws_IAE_if_arg_is_empty() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Key '' must be at least 2 chars long"); + expectedException.expectMessage("Key must not be empty"); underTest.checkKey(""); } @Test - public void checkValidKey_throws_IAE_if_arg_is_1_char_long() { + public void checkValidKey_throws_IAE_if_key_is_empty() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Key 'a' must be at least 2 chars long"); + expectedException.expectMessage("Key must not be empty"); - underTest.checkKey("a"); + underTest.checkKey(""); } @Test @@ -109,21 +109,13 @@ public class OrganizationValidationImplTest { } @Test - public void checkValidName_throws_IAE_if_arg_is_empty() { + public void checkValidName_throws_IAE_if_empty() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Name '' must be at least 2 chars long"); + expectedException.expectMessage("Name must not be empty"); underTest.checkName(""); } - @Test - public void checkValidName_throws_IAE_if_arg_is_1_char_long() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Name 'a' must be at least 2 chars long"); - - underTest.checkName("a"); - } - @Test public void checkValidName_does_not_fail_if_arg_is_2_to_32_chars_long() { String str = "aa"; diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java index 457c5c82d41..c996eaaf8e8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java @@ -23,17 +23,16 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; import javax.annotation.Nullable; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.System2; +import org.sonar.api.utils.internal.TestSystem2; import org.sonar.api.web.UserRole; import org.sonar.core.permission.GlobalPermissions; -import org.sonar.core.util.UuidFactory; -import org.sonar.core.util.Uuids; +import org.sonar.core.util.UuidFactoryFast; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; @@ -67,17 +66,18 @@ import org.sonarqube.ws.Organizations.Organization; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import static org.sonar.core.config.CorePropertyDefinitions.ORGANIZATIONS_ANYONE_CAN_CREATE; +import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_KEY; +import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_NAME; import static org.sonar.server.organization.ws.OrganizationsWsTestSupport.STRING_257_CHARS_LONG; import static org.sonar.server.organization.ws.OrganizationsWsTestSupport.STRING_65_CHARS_LONG; import static org.sonar.test.JsonAssert.assertJson; public class CreateActionTest { - private static final String SOME_UUID = "uuid"; - private static final long SOME_DATE = 1_200_000L; - private System2 system2 = mock(System2.class); + private static final long NOW = 1_200_000L; + + private System2 system2 = new TestSystem2().setNow(NOW); @Rule public UserSessionRule userSession = UserSessionRule.standalone(); @@ -90,29 +90,19 @@ public class CreateActionTest { private DbClient dbClient = dbTester.getDbClient(); private DbSession dbSession = dbTester.getSession(); - private MapSettings settings = new MapSettings() - .setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, false); - private UuidFactory uuidFactory = mock(UuidFactory.class); + private MapSettings settings = new MapSettings().setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, false); private OrganizationValidation organizationValidation = new OrganizationValidationImpl(); private UserIndexer userIndexer = new UserIndexer(dbClient, es.client()); private UserIndex userIndex = new UserIndex(es.client(), System2.INSTANCE); - private OrganizationCreation organizationCreation = new OrganizationCreationImpl(dbClient, system2, uuidFactory, organizationValidation, settings.asConfig(), userIndexer, + private OrganizationCreation organizationCreation = new OrganizationCreationImpl(dbClient, system2, UuidFactoryFast.getInstance(), organizationValidation, settings.asConfig(), + userIndexer, mock(BuiltInQProfileRepository.class), new DefaultGroupCreatorImpl(dbClient)); private TestOrganizationFlags organizationFlags = TestOrganizationFlags.standalone().setEnabled(true); - private UserDto user; - - private CreateAction underTest = new CreateAction(settings.asConfig(), userSession, dbClient, new OrganizationsWsSupport(organizationValidation), organizationValidation, - organizationCreation, organizationFlags); - - private WsActionTester wsTester = new WsActionTester(underTest); - - @Before - public void setUp() { - user = dbTester.users().insertUser(); - userIndexer.indexOnStartup(new HashSet<>()); - userSession.logIn(user); - } + private WsActionTester wsTester = new WsActionTester( + new CreateAction(settings.asConfig(), userSession, dbClient, new OrganizationsWsSupport(organizationValidation), + organizationValidation, + organizationCreation, organizationFlags)); @Test public void verify_define() { @@ -123,7 +113,7 @@ public class CreateActionTest { "Requires 'Administer System' permission unless any logged in user is allowed to create an organization (see appropriate setting). Organization support must be enabled."); assertThat(action.isInternal()).isTrue(); assertThat(action.since()).isEqualTo("6.2"); - assertThat(action.handler()).isEqualTo(underTest); + assertThat(action.handler()).isNotNull(); assertThat(action.params()).hasSize(5); assertThat(action.responseExample()).isEqualTo(getClass().getResource("create-example.json")); assertThat(action.param("name")) @@ -150,8 +140,7 @@ public class CreateActionTest { @Test public void verify_response_example() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(Uuids.UUID_EXAMPLE_01, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); String response = executeJsonRequest("Foo Company", "foo-company", "The Foo company produces quality software for Bar.", "https://www.foo.com", "https://www.foo.com/foo.png"); @@ -159,328 +148,142 @@ public class CreateActionTest { assertJson(response).isSimilarTo(wsTester.getDef().responseExampleAsString()); } - @Test - public void request_fails_if_user_is_not_logged_in_and_logged_in_users_cannot_create_organizations() { - userSession.anonymous(); - - expectedException.expect(ForbiddenException.class); - expectedException.expectMessage("Insufficient privileges"); - - executeRequest("name"); - } - - @Test - public void request_fails_if_user_is_not_logged_in_and_logged_in_users_can_create_organizations() { - userSession.anonymous(); - settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); - - expectedException.expect(UnauthorizedException.class); - expectedException.expectMessage("Authentication is required"); - - executeRequest("name"); - } - - @Test - public void request_fails_if_user_is_not_system_administrator_and_logged_in_users_cannot_create_organizations() { - userSession.logIn(); - - expectedException.expect(ForbiddenException.class); - expectedException.expectMessage("Insufficient privileges"); - - executeRequest("name"); - } - @Test public void request_succeeds_if_user_is_system_administrator_and_logged_in_users_cannot_create_organizations() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); - verifyResponseAndDb(executeRequest("foo"), SOME_UUID, "foo", "foo", SOME_DATE); + verifyResponseAndDb(executeRequest("foo"), "foo", "foo", NOW); } @Test public void request_succeeds_if_user_is_system_administrator_and_logged_in_users_can_create_organizations() { - logInAsSystemAdministrator(); + createUserAndLogInAsSystemAdministrator(); settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); dbTester.qualityGates().insertBuiltInQualityGate(); - verifyResponseAndDb(executeRequest("foo"), SOME_UUID, "foo", "foo", SOME_DATE); + verifyResponseAndDb(executeRequest("foo"), "foo", "foo", NOW); } @Test public void request_succeeds_if_user_is_not_system_administrator_and_logged_in_users_can_create_organizations() { + UserDto user = dbTester.users().insertUser(); userSession.logIn(user); settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); dbTester.qualityGates().insertBuiltInQualityGate(); - verifyResponseAndDb(executeRequest("foo"), SOME_UUID, "foo", "foo", SOME_DATE); - } - - @Test - public void request_fails_if_name_param_is_missing() { - logInAsSystemAdministrator(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("The 'name' parameter is missing"); - - executeRequest(null); - } - - @Test - public void request_fails_if_name_is_one_char_long() { - logInAsSystemAdministrator(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Name 'a' must be at least 2 chars long"); - - executeRequest("a"); + verifyResponseAndDb(executeRequest("foo"), "foo", "foo", NOW); } @Test public void request_succeeds_if_name_is_two_chars_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); - verifyResponseAndDb(executeRequest("ab"), SOME_UUID, "ab", "ab", SOME_DATE); - } - - @Test - public void request_fails_if_name_is_65_chars_long() { - logInAsSystemAdministrator(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("'name' length (65) is longer than the maximum authorized (64)"); - - executeRequest(STRING_65_CHARS_LONG); + verifyResponseAndDb(executeRequest("ab"), "ab", "ab", NOW); } @Test public void request_succeeds_if_name_is_64_char_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); String name = STRING_65_CHARS_LONG.substring(0, 64); - verifyResponseAndDb(executeRequest(name), SOME_UUID, name, name.substring(0, 32), SOME_DATE); - } - - @Test - public void request_fails_if_key_one_char_long() { - logInAsSystemAdministrator(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Key 'a' must be at least 2 chars long"); - - executeRequest("foo", "a"); - } - - @Test - public void request_fails_if_key_is_33_chars_long() { - logInAsSystemAdministrator(); - - String key = STRING_65_CHARS_LONG.substring(0, 33); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("'key' length (33) is longer than the maximum authorized (32)"); - - executeRequest("foo", key); + verifyResponseAndDb(executeRequest(name), name, name.substring(0, 32), NOW); } @Test public void request_succeeds_if_key_is_2_chars_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); - verifyResponseAndDb(executeRequest("foo", "ab"), SOME_UUID, "foo", "ab", SOME_DATE); + verifyResponseAndDb(executeRequest("foo", "ab"), "foo", "ab", NOW); } @Test public void requests_succeeds_if_key_is_32_chars_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); String key = STRING_65_CHARS_LONG.substring(0, 32); - verifyResponseAndDb(executeRequest("foo", key), SOME_UUID, "foo", key, SOME_DATE); - } - - @Test - public void requests_fails_if_key_contains_non_ascii_chars_but_dash() { - logInAsSystemAdministrator(); - - requestFailsWithInvalidCharInKey("ab@"); - } - - @Test - public void request_fails_if_key_starts_with_a_dash() { - logInAsSystemAdministrator(); - - requestFailsWithInvalidCharInKey("-ab"); - } - - @Test - public void request_fails_if_key_ends_with_a_dash() { - logInAsSystemAdministrator(); - - requestFailsWithInvalidCharInKey("ab-"); - } - - @Test - public void request_fails_if_key_contains_space() { - logInAsSystemAdministrator(); - - requestFailsWithInvalidCharInKey("a b"); - } - - private void requestFailsWithInvalidCharInKey(String key) { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Key '" + key + "' contains at least one invalid char"); - - executeRequest("foo", key); - } - - @Test - public void request_fails_if_key_is_specified_and_already_exists_in_DB() { - logInAsSystemAdministrator(); - OrganizationDto org = dbTester.organizations().insert(o -> o.setKey("the-key")); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Key '" + org.getKey() + "' is already used. Specify another one."); - - executeRequest("foo", org.getKey()); - } - - @Test - public void request_fails_if_key_computed_from_name_already_exists_in_DB() { - logInAsSystemAdministrator(); - String key = STRING_65_CHARS_LONG.substring(0, 32); - dbTester.organizations().insert(o -> o.setKey(key)); - - String name = STRING_65_CHARS_LONG.substring(0, 64); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Key '" + key + "' generated from name '" + name + "' is already used. Specify one."); - - executeRequest(name); + verifyResponseAndDb(executeRequest("foo", key), "foo", key, NOW); } @Test public void request_succeeds_if_description_url_and_avatar_are_not_specified() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); CreateWsResponse response = executeRequest("foo", "bar", null, null, null); - verifyResponseAndDb(response, SOME_UUID, "foo", "bar", null, null, null, SOME_DATE); + + verifyResponseAndDb(response, "foo", "bar", null, null, null, NOW); } @Test public void request_succeeds_if_description_url_and_avatar_are_specified() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); CreateWsResponse response = executeRequest("foo", "bar", "moo", "doo", "boo"); - verifyResponseAndDb(response, SOME_UUID, "foo", "bar", "moo", "doo", "boo", SOME_DATE); + verifyResponseAndDb(response, "foo", "bar", "moo", "doo", "boo", NOW); } @Test public void request_succeeds_to_generate_key_from_name_more_then_32_chars_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); String name = STRING_65_CHARS_LONG.substring(0, 33); CreateWsResponse response = executeRequest(name); - verifyResponseAndDb(response, SOME_UUID, name, name.substring(0, 32), SOME_DATE); + verifyResponseAndDb(response, name, name.substring(0, 32), NOW); } @Test public void request_generates_key_ignoring_multiple_following_spaces() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); String name = "ab cd"; CreateWsResponse response = executeRequest(name); - verifyResponseAndDb(response, SOME_UUID, name, "ab-cd", SOME_DATE); - } - - @Test - public void request_fails_if_description_is_257_chars_long() { - logInAsSystemAdministrator(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("'description' length (257) is longer than the maximum authorized (256)"); - - executeRequest("foo", "bar", STRING_257_CHARS_LONG, null, null); + verifyResponseAndDb(response, name, "ab-cd", NOW); } @Test public void request_succeeds_if_description_is_256_chars_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); String description = STRING_257_CHARS_LONG.substring(0, 256); dbTester.qualityGates().insertBuiltInQualityGate(); CreateWsResponse response = executeRequest("foo", "bar", description, null, null); - verifyResponseAndDb(response, SOME_UUID, "foo", "bar", description, null, null, SOME_DATE); - } - - @Test - public void request_fails_if_url_is_257_chars_long() { - logInAsSystemAdministrator(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("'url' length (257) is longer than the maximum authorized (256)"); - - executeRequest("foo", "bar", null, STRING_257_CHARS_LONG, null); + verifyResponseAndDb(response, "foo", "bar", description, null, null, NOW); } @Test public void request_succeeds_if_url_is_256_chars_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); String url = STRING_257_CHARS_LONG.substring(0, 256); dbTester.qualityGates().insertBuiltInQualityGate(); CreateWsResponse response = executeRequest("foo", "bar", null, url, null); - verifyResponseAndDb(response, SOME_UUID, "foo", "bar", null, url, null, SOME_DATE); - } - - @Test - public void request_fails_if_avatar_is_257_chars_long() { - logInAsSystemAdministrator(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("'avatar' length (257) is longer than the maximum authorized (256)"); - - executeRequest("foo", "bar", null, null, STRING_257_CHARS_LONG); + verifyResponseAndDb(response, "foo", "bar", null, url, null, NOW); } @Test public void request_succeeds_if_avatar_is_256_chars_long() { - logInAsSystemAdministrator(); - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); + createUserAndLogInAsSystemAdministrator(); String avatar = STRING_257_CHARS_LONG.substring(0, 256); dbTester.qualityGates().insertBuiltInQualityGate(); CreateWsResponse response = executeRequest("foo", "bar", null, null, avatar); - verifyResponseAndDb(response, SOME_UUID, "foo", "bar", null, null, avatar, SOME_DATE); + verifyResponseAndDb(response, "foo", "bar", null, null, avatar, NOW); } @Test public void request_creates_owners_group_with_all_permissions_for_new_organization_and_add_current_user_to_it() { - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); UserDto user = dbTester.users().insertUser(); userSession.logIn(user).setSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); @@ -509,7 +312,6 @@ public class CreateActionTest { @Test public void request_creates_members_group_and_add_current_user_to_it() { - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); UserDto user = dbTester.users().insertUser(); userSession.logIn(user).setSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); @@ -537,7 +339,6 @@ public class CreateActionTest { @Test public void request_creates_default_template_for_owner_group() { - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); UserDto user = dbTester.users().insertUser(); userSession.logIn(user).setSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); @@ -562,21 +363,217 @@ public class CreateActionTest { @Test public void request_set_user_as_member_of_organization() { - mockForSuccessfulInsert(SOME_UUID, SOME_DATE); UserDto user = dbTester.users().insertUser(); userSession.logIn(user).setSystemAdministrator(); dbTester.qualityGates().insertBuiltInQualityGate(); - executeRequest("orgFoo"); + executeRequest("foo", "bar"); + + OrganizationDto organization = dbClient.organizationDao().selectByKey(dbSession, "bar").get(); + assertThat(dbClient.organizationMemberDao().select(dbSession, organization.getUuid(), user.getId())).isPresent(); + assertThat(userIndex.getNullableByLogin(user.getLogin()).organizationUuids()).contains(organization.getUuid()); + } + + @Test + public void create_organization_with_name_having_one_character() { + createUserAndLogInAsSystemAdministrator(); + dbTester.qualityGates().insertBuiltInQualityGate(); + + wsTester.newRequest() + .setParam(PARAM_NAME, "a") + .execute(); + + OrganizationDto organization = dbClient.organizationDao().selectByKey(dbTester.getSession(), "a").get(); + assertThat(organization.getKey()).isEqualTo("a"); + assertThat(organization.getName()).isEqualTo("a"); + } + + @Test + public void request_fails_if_name_param_is_missing() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("The 'name' parameter is missing"); + + executeRequest(null); + } + + @Test + public void request_fails_if_name_is_empty() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Name must not be empty"); + + wsTester.newRequest() + .setParam(PARAM_NAME, "") + .execute(); + } + + @Test + public void request_fails_if_name_is_65_chars_long() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("'name' length (65) is longer than the maximum authorized (64)"); + + executeRequest(STRING_65_CHARS_LONG); + } + + @Test + public void request_fails_if_key_is_33_chars_long() { + createUserAndLogInAsSystemAdministrator(); + + String key = STRING_65_CHARS_LONG.substring(0, 33); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("'key' length (33) is longer than the maximum authorized (32)"); + + executeRequest("foo", key); + } + + @Test + public void requests_fails_if_key_contains_non_ascii_chars_but_dash() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Key '" + "ab@" + "' contains at least one invalid char"); + + executeRequest("foo", "ab@"); + } + + @Test + public void request_fails_if_key_starts_with_a_dash() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Key '" + "-ab" + "' contains at least one invalid char"); + + executeRequest("foo", "-ab"); + } + + @Test + public void request_fails_if_key_ends_with_a_dash() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Key '" + "ab-" + "' contains at least one invalid char"); + + executeRequest("foo", "ab-"); + } + + @Test + public void request_fails_if_key_contains_space() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Key '" + "a b" + "' contains at least one invalid char"); + + executeRequest("foo", "a b"); + } + + @Test + public void request_fails_if_key_is_empty() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Key must not be empty"); + + wsTester.newRequest() + .setParam(PARAM_KEY, "") + .setParam(PARAM_NAME, "foo") + .execute(); + } + + @Test + public void request_fails_if_key_is_specified_and_already_exists_in_DB() { + createUserAndLogInAsSystemAdministrator(); + OrganizationDto org = dbTester.organizations().insert(o -> o.setKey("the-key")); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Key '" + org.getKey() + "' is already used. Specify another one."); + + executeRequest("foo", org.getKey()); + } + + @Test + public void request_fails_if_key_computed_from_name_already_exists_in_DB() { + createUserAndLogInAsSystemAdministrator(); + String key = STRING_65_CHARS_LONG.substring(0, 32); + dbTester.organizations().insert(o -> o.setKey(key)); + String name = STRING_65_CHARS_LONG.substring(0, 64); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Key '" + key + "' generated from name '" + name + "' is already used. Specify one."); + + executeRequest(name); + } + + @Test + public void request_fails_if_url_is_257_chars_long() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("'url' length (257) is longer than the maximum authorized (256)"); + + executeRequest("foo", "bar", null, STRING_257_CHARS_LONG, null); + } + + @Test + public void request_fails_if_description_is_257_chars_long() { + createUserAndLogInAsSystemAdministrator(); - assertThat(dbClient.organizationMemberDao().select(dbSession, SOME_UUID, user.getId())).isPresent(); - assertThat(userIndex.getNullableByLogin(user.getLogin()).organizationUuids()).contains(SOME_UUID); + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("'description' length (257) is longer than the maximum authorized (256)"); + + executeRequest("foo", "bar", STRING_257_CHARS_LONG, null, null); + } + + @Test + public void request_fails_if_avatar_is_257_chars_long() { + createUserAndLogInAsSystemAdministrator(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("'avatar' length (257) is longer than the maximum authorized (256)"); + + executeRequest("foo", "bar", null, null, STRING_257_CHARS_LONG); + } + + @Test + public void request_fails_if_user_is_not_logged_in_and_logged_in_users_cannot_create_organizations() { + userSession.anonymous(); + + expectedException.expect(ForbiddenException.class); + expectedException.expectMessage("Insufficient privileges"); + + executeRequest("name"); + } + + @Test + public void request_fails_if_user_is_not_logged_in_and_logged_in_users_can_create_organizations() { + userSession.anonymous(); + settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); + + expectedException.expect(UnauthorizedException.class); + expectedException.expectMessage("Authentication is required"); + + executeRequest("name"); + } + + @Test + public void request_fails_if_user_is_not_system_administrator_and_logged_in_users_cannot_create_organizations() { + userSession.logIn(); + + expectedException.expect(ForbiddenException.class); + expectedException.expectMessage("Insufficient privileges"); + + executeRequest("name"); } @Test public void request_fails_with_IllegalStateException_if_organization_support_is_disabled() { organizationFlags.setEnabled(false); - logInAsSystemAdministrator(); + createUserAndLogInAsSystemAdministrator(); expectedException.expect(IllegalStateException.class); expectedException.expectMessage("Organization support is disabled"); @@ -584,11 +581,6 @@ public class CreateActionTest { executeJsonRequest("Foo Company", "foo-company", "The Foo company produces quality software for Bar.", "https://www.foo.com", "https://www.foo.com/foo.png"); } - private void mockForSuccessfulInsert(String uuid, long now) { - when(uuidFactory.create()).thenReturn(uuid); - when(system2.now()).thenReturn(now); - } - private CreateWsResponse executeRequest(@Nullable String name, @Nullable String key) { return executeRequest(name, key, null, null, null); } @@ -619,16 +611,12 @@ public class CreateActionTest { OrganizationsWsTestSupport.setParam(request, "avatar", avatar); } - private void verifyResponseAndDb(CreateWsResponse response, - String uuid, String name, String key, - long createdAt) { - verifyResponseAndDb(response, uuid, name, key, null, null, null, createdAt); + private void verifyResponseAndDb(CreateWsResponse response, String name, String key, long createdAt) { + verifyResponseAndDb(response, name, key, null, null, null, createdAt); } - private void verifyResponseAndDb(CreateWsResponse response, - String id, String name, String key, - @Nullable String description, @Nullable String url, @Nullable String avatar, - long createdAt) { + private void verifyResponseAndDb(CreateWsResponse response, String name, String key, @Nullable String description, @Nullable String url, @Nullable String avatar, + long createdAt) { Organization organization = response.getOrganization(); assertThat(organization.getName()).isEqualTo(name); assertThat(organization.getKey()).isEqualTo(key); @@ -648,8 +636,8 @@ public class CreateActionTest { assertThat(organization.getAvatar()).isEqualTo(avatar); } - OrganizationDto dto = dbClient.organizationDao().selectByUuid(dbTester.getSession(), id).get(); - assertThat(dto.getUuid()).isEqualTo(id); + OrganizationDto dto = dbClient.organizationDao().selectByKey(dbTester.getSession(), key).get(); + assertThat(dto.getUuid()).isNotNull(); assertThat(dto.getKey()).isEqualTo(key); assertThat(dto.getName()).isEqualTo(name); assertThat(dto.getDescription()).isEqualTo(description); @@ -659,7 +647,9 @@ public class CreateActionTest { assertThat(dto.getUpdatedAt()).isEqualTo(createdAt); } - private void logInAsSystemAdministrator() { + private void createUserAndLogInAsSystemAdministrator() { + UserDto user = dbTester.users().insertUser(); + userIndexer.indexOnStartup(new HashSet<>()); userSession.logIn(user).setSystemAdministrator(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java index 8d6e57cc7ff..d4e898dacd1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java @@ -173,13 +173,13 @@ public class UpdateActionTest { } @Test - public void request_fails_if_name_is_one_char_long() { + public void request_fails_if_name_is_empty() { userSession.logIn(); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Name 'a' must be at least 2 chars long"); + expectedException.expectMessage("Name must not be empty"); - executeKeyRequest(SOME_KEY, "a"); + executeKeyRequest(SOME_KEY, ""); } @Test diff --git a/tests/src/test/java/org/sonarqube/tests/organization/OrganizationTest.java b/tests/src/test/java/org/sonarqube/tests/organization/OrganizationTest.java index 85492174721..9951ebe9e27 100644 --- a/tests/src/test/java/org/sonarqube/tests/organization/OrganizationTest.java +++ b/tests/src/test/java/org/sonarqube/tests/organization/OrganizationTest.java @@ -30,6 +30,7 @@ import org.junit.Test; import org.sonarqube.qa.util.OrganizationTester; import org.sonarqube.qa.util.Tester; import org.sonarqube.ws.Components; +import org.sonarqube.ws.Organizations; import org.sonarqube.ws.Organizations.Organization; import org.sonarqube.ws.Qualityprofiles; import org.sonarqube.ws.Rules; @@ -43,7 +44,6 @@ import org.sonarqube.ws.client.organizations.OrganizationsService; import org.sonarqube.ws.client.organizations.SearchRequest; import org.sonarqube.ws.client.organizations.UpdateRequest; import org.sonarqube.ws.client.permissions.AddUserRequest; -import org.sonarqube.ws.client.permissions.PermissionsService; import org.sonarqube.ws.client.roots.SetRootRequest; import org.sonarqube.ws.client.roots.UnsetRootRequest; @@ -192,11 +192,9 @@ public class OrganizationTest { @Test public void an_organization_member_can_analyze_project() { Organization organization = tester.organizations().generate(); - User user = tester.users().generate(); - Group group = tester.groups().generate(organization); - // users.removeGroups("sonar-users"); - tester.organizations().service().addMember(new AddMemberRequest().setOrganization(organization.getKey()).setLogin(user.getLogin())); - addPermissionsToUser(organization.getKey(), user.getLogin(), "provisioning", "scan"); + User user = tester.users().generateMember(organization); + tester.wsClient().permissions().addUser(new AddUserRequest().setLogin(user.getLogin()).setOrganization(organization.getKey()).setPermission("provisioning")); + tester.wsClient().permissions().addUser(new AddUserRequest().setLogin(user.getLogin()).setOrganization(organization.getKey()).setPermission("scan")); runProjectAnalysis(orchestrator, "shared/xoo-sample", "sonar.organization", organization.getKey(), @@ -229,14 +227,6 @@ public class OrganizationTest { assertThat(searchSampleProject(organization.getKey()).getComponentsList()).hasSize(1); } - private void addPermissionsToUser(String orgKeyAndName, String login, String permission, String... otherPermissions) { - PermissionsService permissionsService = tester.wsClient().permissions(); - permissionsService.addUser(new AddUserRequest().setLogin(login).setOrganization(orgKeyAndName).setPermission(permission)); - for (String otherPermission : otherPermissions) { - permissionsService.addUser(new AddUserRequest().setLogin(login).setOrganization(orgKeyAndName).setPermission(otherPermission)); - } - } - @Test public void deleting_an_organization_deletes_its_projects() { Organization organization = tester.organizations().generate(); @@ -292,6 +282,16 @@ public class OrganizationTest { .hasSize(1); } + @Test + public void create_organization_having_name_of_one_character() { + tester.organizations().generate(o -> o.setName("A")); + + Organizations.SearchWsResponse search = tester.organizations().service().search(new SearchRequest()); + + assertThat(search.getOrganizationsList()) + .extracting(Organization::getName).contains("A"); + } + private Components.SearchWsResponse searchSampleProject(String organizationKey) { return tester.wsClient().components() .search(new org.sonarqube.ws.client.components.SearchRequest() diff --git a/tests/src/test/java/org/sonarqube/tests/organization/PersonalOrganizationTest.java b/tests/src/test/java/org/sonarqube/tests/organization/PersonalOrganizationTest.java index d6c9f11fabf..c854fe22f66 100644 --- a/tests/src/test/java/org/sonarqube/tests/organization/PersonalOrganizationTest.java +++ b/tests/src/test/java/org/sonarqube/tests/organization/PersonalOrganizationTest.java @@ -21,6 +21,7 @@ package org.sonarqube.tests.organization; import com.sonar.orchestrator.Orchestrator; import java.util.List; +import org.assertj.core.api.Java6Assertions; import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; @@ -31,11 +32,10 @@ import org.sonarqube.ws.Users; import org.sonarqube.ws.client.organizations.SearchRequest; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Java6Assertions.tuple; public class PersonalOrganizationTest { - private static final String SETTING_CREATE_PERSONAL_ORG = "sonar.organizations.createPersonalOrg"; - @ClassRule public static Orchestrator orchestrator = OrganizationSuite.ORCHESTRATOR; @@ -44,7 +44,7 @@ public class PersonalOrganizationTest { @Before public void setUp() { - tester.settings().setGlobalSettings(SETTING_CREATE_PERSONAL_ORG, "true"); + tester.settings().setGlobalSettings("sonar.organizations.createPersonalOrg", "true"); } @Test @@ -60,4 +60,14 @@ public class PersonalOrganizationTest { tester.organizations().assertThatMemberOf(existing.get(0), user); } + + @Test + public void create_personal_for_user_having_one_character_size_name() { + tester.users().generate(u -> u.setName("A")); + + List organizations = tester.organizations().service().search(new SearchRequest()).getOrganizationsList(); + Java6Assertions.assertThat(organizations) + .extracting(Organizations.Organization::getName, Organizations.Organization::getGuarded) + .contains(tuple("A", true)); + } } -- 2.39.5