diff options
author | Sébastien Lesaint <sebastien.lesaint@sonarsource.com> | 2016-09-28 18:03:07 +0200 |
---|---|---|
committer | Sébastien Lesaint <sebastien.lesaint@sonarsource.com> | 2016-09-29 08:58:21 +0200 |
commit | 490fb33ae2d1fccc2e0f573073a2b5eb558cb5b0 (patch) | |
tree | 8eee688f56a54f5cde282874c5dde0f8b71fbe48 | |
parent | 9a7b16406681a4bea454525a17bd884e0764ed1f (diff) | |
download | sonarqube-490fb33ae2d1fccc2e0f573073a2b5eb558cb5b0.tar.gz sonarqube-490fb33ae2d1fccc2e0f573073a2b5eb558cb5b0.zip |
SONAR-8100 remove return of id and id parameter in organizations WSs
13 files changed, 120 insertions, 412 deletions
diff --git a/it/it-tests/src/test/java/it/organization/OrganizationIt.java b/it/it-tests/src/test/java/it/organization/OrganizationIt.java index d2fee2e5291..2d7b6bfc316 100644 --- a/it/it-tests/src/test/java/it/organization/OrganizationIt.java +++ b/it/it-tests/src/test/java/it/organization/OrganizationIt.java @@ -59,7 +59,6 @@ public class OrganizationIt { .setAvatar(AVATAR_URL) .build()) .getOrganization(); - assertThat(createdOrganization.getId()).isNotNull(); assertThat(createdOrganization.getName()).isEqualTo(NAME); assertThat(createdOrganization.getKey()).isEqualTo(KEY); assertThat(createdOrganization.getDescription()).isEqualTo(DESCRIPTION); @@ -70,7 +69,7 @@ public class OrganizationIt { // update by id adminOrganizationService.update(new UpdateWsRequest.Builder() - .setId(createdOrganization.getId()) + .setKey(createdOrganization.getKey()) .setName("new name") .setDescription("new description") .setUrl("new url") @@ -80,7 +79,7 @@ public class OrganizationIt { // update by key adminOrganizationService.update(new UpdateWsRequest.Builder() - .setId(createdOrganization.getId()) + .setKey(createdOrganization.getKey()) .setName("new name 2") .setDescription("new description 2") .setUrl("new url 2") @@ -90,13 +89,13 @@ public class OrganizationIt { // remove optional fields adminOrganizationService.update(new UpdateWsRequest.Builder() - .setId(createdOrganization.getId()) + .setKey(createdOrganization.getKey()) .setName("new name 3") .build()); verifySingleSearchResult(createdOrganization, "new name 3", null, null, null); - // delete by uuid - adminOrganizationService.delete(createdOrganization.getId(), null); + // delete organization + adminOrganizationService.delete(createdOrganization.getKey()); verifyNoExtraOrganization(); } @@ -110,10 +109,6 @@ public class OrganizationIt { .getOrganization(); assertThat(createdOrganization.getKey()).isEqualTo("foo-company-to-keyize"); verifySingleSearchResult(createdOrganization, name, null, null, null); - - // delete by key - adminOrganizationService.delete(null, createdOrganization.getKey()); - verifyNoExtraOrganization(); } private void verifyNoExtraOrganization() { @@ -131,7 +126,6 @@ public class OrganizationIt { .filter(organization -> !DEFAULT_ORGANIZATION_KEY.equals(organization.getKey())) .findFirst() .get(); - assertThat(searchedOrganization.getId()).isEqualTo(createdOrganization.getId()); assertThat(searchedOrganization.getKey()).isEqualTo(createdOrganization.getKey()); assertThat(searchedOrganization.getName()).isEqualTo(name); if (description == null) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java index 49fd78a4bee..3e0cb0672d1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java @@ -31,21 +31,16 @@ import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.user.UserSession; import static com.google.common.base.Preconditions.checkArgument; -import static java.lang.String.format; -import static org.sonar.core.util.Uuids.UUID_EXAMPLE_03; -import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_ID; import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_KEY; public class DeleteAction implements OrganizationsAction { private static final String ACTION = "delete"; - private final OrganizationsWsSupport wsSupport; private final UserSession userSession; private final DbClient dbClient; private final DefaultOrganizationProvider defaultOrganizationProvider; - public DeleteAction(OrganizationsWsSupport wsSupport, UserSession userSession, DbClient dbClient, DefaultOrganizationProvider defaultOrganizationProvider) { - this.wsSupport = wsSupport; + public DeleteAction(UserSession userSession, DbClient dbClient, DefaultOrganizationProvider defaultOrganizationProvider) { this.userSession = userSession; this.dbClient = dbClient; this.defaultOrganizationProvider = defaultOrganizationProvider; @@ -55,22 +50,14 @@ public class DeleteAction implements OrganizationsAction { public void define(WebService.NewController context) { WebService.NewAction action = context.createAction(ACTION) .setPost(true) - .setDescription( - format("Delete an organization.<br/>" + - "The '%s' or '%s' must be provided.<br/>" + - "Require 'Administer System' permission.", - PARAM_ID, PARAM_KEY)) + .setDescription("Delete an organization.<br/>" + + "Require 'Administer System' permission.") .setInternal(true) .setSince("6.2") .setHandler(this); - action.createParam(PARAM_ID) - .setRequired(false) - .setDescription("Organization id") - .setExampleValue(UUID_EXAMPLE_03); - action.createParam(PARAM_KEY) - .setRequired(false) + .setRequired(true) .setDescription("Organization key") .setExampleValue("foo-company"); } @@ -79,25 +66,18 @@ public class DeleteAction implements OrganizationsAction { public void handle(Request request, Response response) throws Exception { userSession.checkPermission(GlobalPermissions.SYSTEM_ADMIN); - String uuid = request.param(PARAM_ID); - String key = request.param(PARAM_KEY); - wsSupport.checkKeyOrId(uuid, key); - preventDeletionOfDefaultOrganization(uuid, key, defaultOrganizationProvider.get()); + String key = request.mandatoryParam(PARAM_KEY); + preventDeletionOfDefaultOrganization(key, defaultOrganizationProvider.get()); try (DbSession dbSession = dbClient.openSession(false)) { - if (uuid != null) { - dbClient.organizationDao().deleteByUuid(dbSession, uuid); - } else { - dbClient.organizationDao().deleteByKey(dbSession, key); - } + dbClient.organizationDao().deleteByKey(dbSession, key); dbSession.commit(); response.noContent(); } } - private static void preventDeletionOfDefaultOrganization(@Nullable String uuid, @Nullable String key, DefaultOrganization defaultOrganization) { - checkArgument(uuid == null || !defaultOrganization.getUuid().equals(uuid), "Default Organization can't be deleted"); + private static void preventDeletionOfDefaultOrganization(@Nullable String key, DefaultOrganization defaultOrganization) { checkArgument(key == null || !defaultOrganization.getKey().equals(key), "Default Organization can't be deleted"); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java index 73e13664fce..1f87799523a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java @@ -20,7 +20,6 @@ package org.sonar.server.organization.ws; import javax.annotation.CheckForNull; -import javax.annotation.Nullable; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.WebService; import org.sonar.db.organization.OrganizationDto; @@ -32,7 +31,6 @@ import static com.google.common.base.Preconditions.checkArgument; * Factorizes code and constants between Organization WS's actions. */ public class OrganizationsWsSupport { - static final String PARAM_ID = "id"; static final String PARAM_KEY = "key"; static final String PARAM_NAME = "name"; static final String PARAM_DESCRIPTION = "description"; @@ -45,10 +43,6 @@ public class OrganizationsWsSupport { static final int DESCRIPTION_MAX_LENGTH = 256; static final int URL_MAX_LENGTH = 256; - void checkKeyOrId(@Nullable String uuid, @Nullable String key) { - checkArgument(uuid != null ^ key != null, "Either '%s' or '%s' must be provided, not both", PARAM_ID, PARAM_KEY); - } - String getAndCheckName(Request request) { String name = request.mandatoryParam(PARAM_NAME); checkArgument(name.length() >= NAME_MIN_LENGTH, "Name '%s' must be at least %s chars long", name, NAME_MIN_LENGTH); @@ -109,7 +103,6 @@ public class OrganizationsWsSupport { Organizations.Organization toOrganization(Organizations.Organization.Builder builder, OrganizationDto dto) { builder - .setId(dto.getUuid()) .setName(dto.getName()) .setKey(dto.getKey()); if (dto.getDescription() != null) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java index e44c6bec719..91d4e3f28d9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java @@ -20,7 +20,6 @@ package org.sonar.server.organization.ws; import java.util.Optional; -import javax.annotation.Nullable; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; @@ -33,9 +32,7 @@ import org.sonar.server.user.UserSession; import org.sonarqube.ws.Organizations; import static java.lang.String.format; -import static org.sonar.core.util.Uuids.UUID_EXAMPLE_03; import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_KEY; -import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_ID; import static org.sonar.server.ws.WsUtils.writeProtobuf; public class UpdateAction implements OrganizationsAction { @@ -55,22 +52,14 @@ public class UpdateAction implements OrganizationsAction { public void define(WebService.NewController context) { WebService.NewAction action = context.createAction(ACTION) .setPost(true) - .setDescription( - format("Update an organization.<br/>" + - "The '%s' or '%s' must be provided.<br/>" + - "Require 'Administer System' permission.", - PARAM_ID, PARAM_KEY)) + .setDescription("Update an organization.<br/>" + + "Require 'Administer System' permission.") .setInternal(true) .setSince("6.2") .setHandler(this); - action.createParam(PARAM_ID) - .setRequired(false) - .setDescription("Organization id") - .setExampleValue(UUID_EXAMPLE_03); - action.createParam(PARAM_KEY) - .setRequired(false) + .setRequired(true) .setDescription("Organization key") .setExampleValue("foo-company"); @@ -81,16 +70,14 @@ public class UpdateAction implements OrganizationsAction { public void handle(Request request, Response response) throws Exception { userSession.checkPermission(GlobalPermissions.SYSTEM_ADMIN); - String uuid = request.param(PARAM_ID); - String key = request.param(PARAM_KEY); - wsSupport.checkKeyOrId(uuid, key); + String key = request.mandatoryParam(PARAM_KEY); String name = wsSupport.getAndCheckName(request); String description = wsSupport.getAndCheckDescription(request); String url = wsSupport.getAndCheckUrl(request); String avatar = wsSupport.getAndCheckAvatar(request); try (DbSession dbSession = dbClient.openSession(false)) { - OrganizationDto dto = getDto(dbSession, uuid, key); + OrganizationDto dto = getDto(dbSession, key); dto.setName(name) .setDescription(description) .setUrl(url) @@ -102,17 +89,10 @@ public class UpdateAction implements OrganizationsAction { } } - private OrganizationDto getDto(DbSession dbSession, @Nullable String uuid, @Nullable String key) { - if (uuid != null) { - return failIfEmpty(dbClient.organizationDao().selectByUuid(dbSession, uuid), "Organization not found for uuid '%s'", uuid); - } else { - return failIfEmpty(dbClient.organizationDao().selectByKey(dbSession, key), "Organization not found for key '%s'", key); - } - } - - private static OrganizationDto failIfEmpty(Optional<OrganizationDto> organizationDto, String msg, Object argument) { + private OrganizationDto getDto(DbSession dbSession, String key) { + Optional<OrganizationDto> organizationDto = dbClient.organizationDao().selectByKey(dbSession, key); if (!organizationDto.isPresent()) { - throw new NotFoundException(format(msg, argument)); + throw new NotFoundException(format("Organization not found for key '%s'", (Object) key)); } return organizationDto.get(); } diff --git a/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-create.json b/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-create.json index 5cfe18f9c7c..a350af2f7a4 100644 --- a/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-create.json +++ b/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-create.json @@ -1,6 +1,5 @@ { "organization": { - "id": "AU-Tpxb--iU5OvuD2FLy", "key": "foo-company", "name": "Foo Company", "description": "The Foo company produces quality software for Bar.", diff --git a/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-search.json b/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-search.json index 2377c2ba93b..4d26e008228 100644 --- a/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-search.json +++ b/server/sonar-server/src/main/resources/org/sonar/server/organization/ws/example-search.json @@ -1,7 +1,6 @@ { "organizations": [ { - "id": "AU-Tpxb--iU5OvuD2FLy", "key": "foo-company", "name": "Foo Company", "description": "The Foo company produces quality software.", @@ -9,7 +8,6 @@ "avatar": "https://www.foo.com/foo.png" }, { - "id": "AU-TpxcA-iU5OvuD2FLz", "key": "bar-company", "name": "Bar Company", "description": "The Bar company produces quality software too.", 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 ce88d53ba46..7221a1578df 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 @@ -420,7 +420,6 @@ public class CreateActionTest { @Nullable String description, @Nullable String url, @Nullable String avatar, long createdAt) { Organization organization = response.getOrganization(); - assertThat(organization.getId()).isEqualTo(id); assertThat(organization.getName()).isEqualTo(name); assertThat(organization.getKey()).isEqualTo(key); if (description == null) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java index c3df3a3872b..6162641bdfc 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.organization.ws; -import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -36,15 +35,13 @@ import org.sonar.server.ws.WsActionTester; import org.sonarqube.ws.MediaTypes; import static org.assertj.core.api.Assertions.assertThat; -import static org.sonar.core.util.Uuids.UUID_EXAMPLE_03; import static org.sonar.server.organization.DefaultOrganizationProviderRule.someDefaultOrganization; import static org.sonar.server.organization.ws.OrganizationsWsTestSupport.setParam; public class DeleteActionTest { - private static final String SOME_UUID = "uuid"; private static final String SOME_KEY = "key"; private static final int HTTP_CODE_NO_CONTENT = 204; - public static final String ORGANIZATIONS_TABLE = "organizations"; + private static final String ORGANIZATIONS_TABLE = "organizations"; @Rule public UserSessionRule userSession = UserSessionRule.standalone(); @@ -55,7 +52,7 @@ public class DeleteActionTest { @Rule public DefaultOrganizationProviderRule defaultOrganizationProvider = someDefaultOrganization(); - private DeleteAction underTest = new DeleteAction(new OrganizationsWsSupport(), userSession, dbTester.getDbClient(), defaultOrganizationProvider); + private DeleteAction underTest = new DeleteAction(userSession, dbTester.getDbClient(), defaultOrganizationProvider); private WsActionTester wsTester = new WsActionTester(underTest); @Test @@ -64,20 +61,15 @@ public class DeleteActionTest { assertThat(action.key()).isEqualTo("delete"); assertThat(action.isPost()).isTrue(); assertThat(action.description()).isEqualTo("Delete an organization.<br/>" + - "The 'id' or 'key' must be provided.<br/>" + "Require 'Administer System' permission."); assertThat(action.isInternal()).isTrue(); assertThat(action.since()).isEqualTo("6.2"); assertThat(action.handler()).isEqualTo(underTest); - assertThat(action.params()).hasSize(2); + assertThat(action.params()).hasSize(1); assertThat(action.responseExample()).isNull(); - assertThat(action.param("id")) - .matches(param -> !param.isRequired()) - .matches(param -> UUID_EXAMPLE_03.equals(param.exampleValue())) - .matches(param -> "Organization id".equals(param.description())); assertThat(action.param("key")) - .matches(param -> !param.isRequired()) + .matches(param -> param.isRequired()) .matches(param -> "foo-company".equals(param.exampleValue())) .matches(param -> "Organization key".equals(param.description())); } @@ -86,121 +78,72 @@ public class DeleteActionTest { public void request_fails_if_user_does_not_have_SYSTEM_ADMIN_permission() { expectInsufficientPrivilegesFE(); - executeIdRequest(SOME_UUID); + executeRequest(SOME_KEY); } @Test public void request_fails_if_user_does_not_have_SYSTEM_ADMIN_permission_when_key_is_specified() { expectInsufficientPrivilegesFE(); - executeKeyRequest(SOME_KEY); + executeRequest(SOME_KEY); } @Test - public void request_fails_if_both_uuid_and_key_are_missing() { + public void request_fails_if_key_is_missing() { giveUserSystemAdminPermission(); - expectIdAndKeySetOrMissingIAE(); - - executeRequest(null, null); - } - - @Test - public void request_fails_if_both_uuid_and_key_are_provided() { - giveUserSystemAdminPermission(); - - expectIdAndKeySetOrMissingIAE(); - - executeRequest(SOME_UUID, SOME_KEY); - } - - private void expectIdAndKeySetOrMissingIAE() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Either 'id' or 'key' must be provided, not both"); - } + expectedException.expectMessage("The 'key' parameter is missing"); - @Test - public void request_does_not_fail_if_table_is_empty_when_specifying_a_uuid() { - giveUserSystemAdminPermission(); - - assertThat(executeIdRequest("another uuid")).isEqualTo(HTTP_CODE_NO_CONTENT); + executeRequest(null); } @Test - public void request_does_not_fail_if_table_is_empty_when_specifying_a_key() { + public void request_does_not_fail_if_table_is_empty() { giveUserSystemAdminPermission(); - assertThat(executeKeyRequest("another key")).isEqualTo(HTTP_CODE_NO_CONTENT); - } - - @Test - public void request_does_not_fail_if_row_with_specified_uuid_does_not_exist() { - giveUserSystemAdminPermission(); - insertOrganization(SOME_UUID); - - assertThat(executeIdRequest("another uuid")).isEqualTo(HTTP_CODE_NO_CONTENT); - assertThat(dbTester.countRowsOfTable(ORGANIZATIONS_TABLE)).isEqualTo(1); + assertThat(executeRequest("another key")).isEqualTo(HTTP_CODE_NO_CONTENT); } @Test public void request_does_not_fail_if_row_with_specified_key_does_not_exist() { giveUserSystemAdminPermission(); - insertOrganization(SOME_UUID); + insertOrganization(SOME_KEY); - assertThat(executeKeyRequest("another key")).isEqualTo(HTTP_CODE_NO_CONTENT); - assertThat(dbTester.countRowsOfTable(ORGANIZATIONS_TABLE)).isEqualTo(1); - } - - @Test - public void request_succeeds_and_delete_row_with_specified_existing_uuid() { - giveUserSystemAdminPermission(); - insertOrganization(SOME_UUID); - insertOrganization("other uuid"); - - assertThat(executeIdRequest(SOME_UUID)).isEqualTo(HTTP_CODE_NO_CONTENT); + assertThat(executeRequest("another key")).isEqualTo(HTTP_CODE_NO_CONTENT); assertThat(dbTester.countRowsOfTable(ORGANIZATIONS_TABLE)).isEqualTo(1); } @Test public void request_succeeds_and_delete_row_with_specified_existing_key() { giveUserSystemAdminPermission(); - String key = insertOrganization(SOME_UUID).getKey(); - insertOrganization("other uuid"); + insertOrganization(SOME_KEY); + insertOrganization("other key"); - assertThat(executeKeyRequest(key)).isEqualTo(HTTP_CODE_NO_CONTENT); + assertThat(executeRequest(SOME_KEY)).isEqualTo(HTTP_CODE_NO_CONTENT); assertThat(dbTester.countRowsOfTable(ORGANIZATIONS_TABLE)).isEqualTo(1); } @Test - public void request_fails_when_attempting_to_delete_Default_Organization_by_uuid() { - giveUserSystemAdminPermission(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Default Organization can't be deleted"); - - executeIdRequest(defaultOrganizationProvider.get().getUuid()); - } - - @Test - public void request_fails_when_attempting_to_delete_Default_Organization_by_key() { + public void request_fails_when_attempting_to_delete_Default_Organization() { giveUserSystemAdminPermission(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Default Organization can't be deleted"); - executeKeyRequest(defaultOrganizationProvider.get().getKey()); + executeRequest(defaultOrganizationProvider.get().getKey()); } - private OrganizationDto insertOrganization(String uuid) { + private OrganizationDto insertOrganization(String key) { OrganizationDto dto = new OrganizationDto() - .setUuid(uuid) - .setKey(uuid + "_key") - .setName(uuid + "_name") - .setDescription(uuid + "_description") - .setUrl(uuid + "_url") - .setAvatarUrl(uuid + "_avatar_url") - .setCreatedAt((long) uuid.hashCode()) - .setUpdatedAt((long) uuid.hashCode()); + .setUuid(key + "_uuid") + .setKey(key) + .setName(key + "_name") + .setDescription(key + "_description") + .setUrl(key + "_url") + .setAvatarUrl(key + "_avatar_url") + .setCreatedAt((long) key.hashCode()) + .setUpdatedAt((long) key.hashCode()); dbTester.getDbClient().organizationDao().insert(dbTester.getSession(), dto); dbTester.commit(); return dto; @@ -215,18 +158,9 @@ public class DeleteActionTest { expectedException.expectMessage("Insufficient privileges"); } - private int executeIdRequest(String id) { - return executeRequest(id, null); - } - - private int executeKeyRequest(String key) { - return executeRequest(null, key); - } - - private int executeRequest(@Nullable String id, @Nullable String key) { + private int executeRequest(String key) { TestRequest request = wsTester.newRequest() .setMediaType(MediaTypes.PROTOBUF); - setParam(request, "id", id); setParam(request, "key", key); return request.execute().getStatus(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/SearchActionTest.java index 7f0e33c1def..aa00075daa4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/SearchActionTest.java @@ -41,7 +41,6 @@ import org.sonarqube.ws.Organizations; import static java.lang.String.valueOf; 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.test.JsonAssert.assertJson; @@ -56,7 +55,7 @@ public class SearchActionTest { .setAvatarUrl("the avatar url") .setCreatedAt(1_999_000L) .setUpdatedAt(1_888_000L); - public static final long SOME_DATE = 1_999_999L; + private static final long SOME_DATE = 1_999_999L; private System2 system2 = mock(System2.class); @@ -145,34 +144,34 @@ public class SearchActionTest { insertOrganization(ORGANIZATION_DTO.setUuid("uuid4").setKey("key-4")); assertThat(executeRequest(1, 1)) - .extracting("id", "key") - .containsExactly(tuple("uuid4", "key-4")); + .extracting("key") + .containsExactly("key-4"); assertThat(executeRequest(2, 1)) - .extracting("id", "key") - .containsExactly(tuple("uuid5", "key-5")); + .extracting("key") + .containsExactly("key-5"); assertThat(executeRequest(3, 1)) - .extracting("id", "key") - .containsExactly(tuple("uuid2", "key-2")); + .extracting("key") + .containsExactly("key-2"); assertThat(executeRequest(4, 1)) - .extracting("id", "key") - .containsExactly(tuple("uuid1", "key-1")); + .extracting("key") + .containsExactly("key-1"); assertThat(executeRequest(5, 1)) - .extracting("id", "key") - .containsExactly(tuple("uuid3", "key-3")); + .extracting("key") + .containsExactly("key-3"); assertThat(executeRequest(6, 1)) .isEmpty(); assertThat(executeRequest(1, 5)) - .extracting("id") - .containsExactly("uuid4", "uuid5", "uuid2", "uuid1", "uuid3"); + .extracting("key") + .containsExactly("key-4", "key-5", "key-2", "key-1", "key-3"); assertThat(executeRequest(2, 5)) .isEmpty(); assertThat(executeRequest(1, 3)) - .extracting("id") - .containsExactly("uuid4", "uuid5", "uuid2"); + .extracting("key") + .containsExactly("key-4", "key-5", "key-2"); assertThat(executeRequest(2, 3)) - .extracting("id") - .containsExactly("uuid1", "uuid3"); + .extracting("key") + .containsExactly("key-1", "key-3"); } private void insertOrganization(OrganizationDto dto) { 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 8ad59eb20bb..6282cbd88d0 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 @@ -39,13 +39,11 @@ import org.sonarqube.ws.Organizations; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.sonar.core.util.Uuids.UUID_EXAMPLE_03; 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.server.organization.ws.OrganizationsWsTestSupport.setParam; public class UpdateActionTest { - private static final String SOME_UUID = "uuid"; private static final String SOME_KEY = "key"; private static final long SOME_DATE = 1_200_000L; @@ -67,20 +65,15 @@ public class UpdateActionTest { assertThat(action.key()).isEqualTo("update"); assertThat(action.isPost()).isTrue(); assertThat(action.description()).isEqualTo("Update an organization.<br/>" + - "The 'id' or 'key' must be provided.<br/>" + "Require 'Administer System' permission."); assertThat(action.isInternal()).isTrue(); assertThat(action.since()).isEqualTo("6.2"); assertThat(action.handler()).isEqualTo(underTest); - assertThat(action.params()).hasSize(6); + assertThat(action.params()).hasSize(5); assertThat(action.responseExample()).isNull(); - assertThat(action.param("id")) - .matches(param -> !param.isRequired()) - .matches(param -> UUID_EXAMPLE_03.equals(param.exampleValue())) - .matches(param -> "Organization id".equals(param.description())); assertThat(action.param("key")) - .matches(param -> !param.isRequired()) + .matches(param -> param.isRequired()) .matches(param -> "foo-company".equals(param.exampleValue())) .matches(param -> "Organization key".equals(param.description())); assertThat(action.param("name")) @@ -106,106 +99,49 @@ public class UpdateActionTest { expectedException.expect(ForbiddenException.class); expectedException.expectMessage("Insufficient privileges"); - executeIdRequest("uuid", "name"); - } - - @Test - public void request_fails_if_both_uuid_and_key_are_missing() { - giveUserSystemAdminPermission(); - - expectUuidAndKeySetOrMissingIAE(); - - executeRequest(null, null, "name", "description", "url", "avatar"); + executeKeyRequest("key", "name"); } @Test - public void request_fails_if_both_uuid_and_key_are_provided() { + public void request_fails_if_key_is_missing() { giveUserSystemAdminPermission(); - expectUuidAndKeySetOrMissingIAE(); - - executeRequest("uuid", "key", "name", "description", "url", "avatar"); - } - - private void expectUuidAndKeySetOrMissingIAE() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Either 'id' or 'key' must be provided, not both"); - } + expectedException.expectMessage("The 'key' parameter is missing"); - @Test - public void request_fails_if_name_param_is_missing_when_uuid_is_provided() { - giveUserSystemAdminPermission(); - - expectMissingNameIAE(); - - executeIdRequest("uuid", null); + executeRequest(null, "name", "description", "url", "avatar"); } @Test - public void request_fails_if_name_param_is_missing_when_key_is_provided() { + public void request_fails_if_name_param_is_missing() { giveUserSystemAdminPermission(); - expectMissingNameIAE(); - - executeKeyRequest("key", null); - } - - private void expectMissingNameIAE() { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The 'name' parameter is missing"); - } - @Test - public void request_fails_if_name_is_one_char_long_when_uuid_is_provided() { - giveUserSystemAdminPermission(); - - expectNameTooShortIAE(); - - executeIdRequest(SOME_UUID, "a"); + executeKeyRequest("key", null); } @Test - public void request_fails_if_name_is_one_char_long_when_key_is_provided() { + public void request_fails_if_name_is_one_char_long() { giveUserSystemAdminPermission(); - expectNameTooShortIAE(); - - executeKeyRequest(SOME_KEY, "a"); - } - - private void expectNameTooShortIAE() { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Name 'a' must be at least 2 chars long"); - } - - @Test - public void request_succeeds_if_name_is_two_chars_long_when_uuid_is_provided() { - giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); - verifyResponseAndDb(executeIdRequest(SOME_UUID, "ab"), dto, "ab", SOME_DATE); - } - - @Test - public void request_succeeds_if_name_is_two_chars_long_when_key_is_provided() { - giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); - - verifyResponseAndDb(executeKeyRequest(dto.getKey(), "ab"), dto, "ab", SOME_DATE); + executeKeyRequest(SOME_KEY, "a"); } @Test - public void request_fails_if_name_is_65_chars_long_when_uuid_is_provided() { + public void request_succeeds_if_name_is_two_chars_long() { giveUserSystemAdminPermission(); + OrganizationDto dto = mockForSuccessfulUpdate(SOME_KEY, SOME_DATE); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Name '" + STRING_65_CHARS_LONG + "' must be at most 64 chars long"); - - executeIdRequest(SOME_UUID, STRING_65_CHARS_LONG); + verifyResponseAndDb(executeKeyRequest(SOME_KEY, "ab"), dto, "ab", SOME_DATE); } @Test - public void request_fails_if_name_is_65_chars_long_when_key_is_provided() { + public void request_fails_if_name_is_65_chars_long() { giveUserSystemAdminPermission(); expectedException.expect(IllegalArgumentException.class); @@ -215,63 +151,35 @@ public class UpdateActionTest { } @Test - public void request_succeeds_if_name_is_64_char_long_when_uuid_is_provided() { + public void request_succeeds_if_name_is_64_char_long() { giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); + OrganizationDto dto = mockForSuccessfulUpdate(SOME_KEY, SOME_DATE); String name = STRING_65_CHARS_LONG.substring(0, 64); - verifyResponseAndDb(executeIdRequest(SOME_UUID, name), dto, name, SOME_DATE); + verifyResponseAndDb(executeKeyRequest(SOME_KEY, name), dto, name, SOME_DATE); } @Test - public void request_succeeds_if_description_url_and_avatar_are_not_specified_when_uuid_is_specified() { + public void request_succeeds_if_description_url_and_avatar_are_not_specified() { giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); + OrganizationDto dto = mockForSuccessfulUpdate(SOME_KEY, SOME_DATE); - Organizations.UpdateWsResponse response = executeIdRequest(SOME_UUID, "bar", null, null, null); + Organizations.UpdateWsResponse response = executeKeyRequest(SOME_KEY, "bar", null, null, null); verifyResponseAndDb(response, dto, "bar", null, null, null, SOME_DATE); } @Test - public void request_succeeds_if_description_url_and_avatar_are_not_specified_when_key_is_specified() { + public void request_succeeds_if_description_url_and_avatar_are_specified() { giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); + OrganizationDto dto = mockForSuccessfulUpdate(SOME_KEY, SOME_DATE); - Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, null, null); - verifyResponseAndDb(response, dto, "bar", null, null, null, SOME_DATE); - } - - @Test - public void request_succeeds_if_description_url_and_avatar_are_specified_when_uuid_is_specified() { - giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); - - Organizations.UpdateWsResponse response = executeIdRequest(SOME_UUID, "bar", "moo", "doo", "boo"); + Organizations.UpdateWsResponse response = executeKeyRequest(SOME_KEY, "bar", "moo", "doo", "boo"); verifyResponseAndDb(response, dto, "bar", "moo", "doo", "boo", SOME_DATE); } @Test - public void request_succeeds_if_description_url_and_avatar_are_specified_when_key_is_specified() { - giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); - - Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", "moo", "doo", "boo"); - verifyResponseAndDb(response, dto, "bar", "moo", "doo", "boo", SOME_DATE); - } - - @Test - public void request_fails_if_description_is_257_chars_long_when_uuid_is_specified() { - giveUserSystemAdminPermission(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("description '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); - - executeIdRequest(SOME_UUID, "bar", STRING_257_CHARS_LONG, null, null); - } - - @Test - public void request_fails_if_description_is_257_chars_long_when_key_is_specified() { + public void request_fails_if_description_is_257_chars_long() { giveUserSystemAdminPermission(); expectedException.expect(IllegalArgumentException.class); @@ -281,37 +189,17 @@ public class UpdateActionTest { } @Test - public void request_succeeds_if_description_is_256_chars_long_when_uuid_is_specified() { - giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); - String description = STRING_257_CHARS_LONG.substring(0, 256); - - Organizations.UpdateWsResponse response = executeIdRequest(SOME_UUID, "bar", description, null, null); - verifyResponseAndDb(response, dto, "bar", description, null, null, SOME_DATE); - } - - @Test - public void request_succeeds_if_description_is_256_chars_long_when_key_is_specified() { + public void request_succeeds_if_description_is_256_chars_long() { giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); + OrganizationDto dto = mockForSuccessfulUpdate(SOME_KEY, SOME_DATE); String description = STRING_257_CHARS_LONG.substring(0, 256); - Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", description, null, null); + Organizations.UpdateWsResponse response = executeKeyRequest(SOME_KEY, "bar", description, null, null); verifyResponseAndDb(response, dto, "bar", description, null, null, SOME_DATE); } @Test - public void request_fails_if_url_is_257_chars_long_when_uuid_is_specified() { - giveUserSystemAdminPermission(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("url '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); - - executeIdRequest(SOME_UUID, "bar", null, STRING_257_CHARS_LONG, null); - } - - @Test - public void request_fails_if_url_is_257_chars_long_when_key_is_specified() { + public void request_fails_if_url_is_257_chars_long_when() { giveUserSystemAdminPermission(); expectedException.expect(IllegalArgumentException.class); @@ -321,37 +209,17 @@ public class UpdateActionTest { } @Test - public void request_succeeds_if_url_is_256_chars_long_when_uuid_is_specified() { - giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); - String url = STRING_257_CHARS_LONG.substring(0, 256); - - Organizations.UpdateWsResponse response = executeIdRequest(SOME_UUID, "bar", null, url, null); - verifyResponseAndDb(response, dto, "bar", null, url, null, SOME_DATE); - } - - @Test - public void request_succeeds_if_url_is_256_chars_long_when_key_is_specified() { + public void request_succeeds_if_url_is_256_chars_long() { giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); + OrganizationDto dto = mockForSuccessfulUpdate(SOME_KEY, SOME_DATE); String url = STRING_257_CHARS_LONG.substring(0, 256); - Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, url, null); + Organizations.UpdateWsResponse response = executeKeyRequest(SOME_KEY, "bar", null, url, null); verifyResponseAndDb(response, dto, "bar", null, url, null, SOME_DATE); } @Test - public void request_fails_if_avatar_is_257_chars_long_when_uuid_is_specified() { - giveUserSystemAdminPermission(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("avatar '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); - - executeIdRequest(SOME_UUID, "bar", null, null, STRING_257_CHARS_LONG); - } - - @Test - public void request_fails_if_avatar_is_257_chars_long_when_key_is_specified() { + public void request_fails_if_avatar_is_257_chars_long() { giveUserSystemAdminPermission(); expectedException.expect(IllegalArgumentException.class); @@ -361,22 +229,12 @@ public class UpdateActionTest { } @Test - public void request_succeeds_if_avatar_is_256_chars_long_when_uuid_is_specified() { + public void request_succeeds_if_avatar_is_256_chars_long() { giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); + OrganizationDto dto = mockForSuccessfulUpdate(SOME_KEY, SOME_DATE); String avatar = STRING_257_CHARS_LONG.substring(0, 256); - Organizations.UpdateWsResponse response = executeIdRequest(SOME_UUID, "bar", null, null, avatar); - verifyResponseAndDb(response, dto, "bar", null, null, avatar, SOME_DATE); - } - - @Test - public void request_succeeds_if_avatar_is_256_chars_long_when_key_is_specified() { - giveUserSystemAdminPermission(); - OrganizationDto dto = mockForSuccessfulUpdate(SOME_UUID, SOME_DATE); - String avatar = STRING_257_CHARS_LONG.substring(0, 256); - - Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, null, avatar); + Organizations.UpdateWsResponse response = executeKeyRequest(SOME_KEY, "bar", null, null, avatar); verifyResponseAndDb(response, dto, "bar", null, null, avatar, SOME_DATE); } @@ -384,49 +242,39 @@ public class UpdateActionTest { userSession.setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); } - private OrganizationDto mockForSuccessfulUpdate(String uuid, long nextNow) { - OrganizationDto dto = insertOrganization(uuid); + private OrganizationDto mockForSuccessfulUpdate(String key, long nextNow) { + OrganizationDto dto = insertOrganization(key); when(system2.now()).thenReturn(nextNow); return dto; } - private OrganizationDto insertOrganization(String uuid) { - when(system2.now()).thenReturn((long) uuid.hashCode()); + private OrganizationDto insertOrganization(String key) { + when(system2.now()).thenReturn((long) key.hashCode()); OrganizationDto dto = new OrganizationDto() - .setUuid(uuid) - .setKey(uuid + "_key") - .setName(uuid + "_name") - .setDescription(uuid + "_description") - .setUrl(uuid + "_url") - .setAvatarUrl(uuid + "_avatar_url"); + .setUuid(key + "_uuid") + .setKey(key) + .setName(key + "_name") + .setDescription(key + "_description") + .setUrl(key + "_url") + .setAvatarUrl(key + "_avatar_url"); dbTester.getDbClient().organizationDao().insert(dbTester.getSession(), dto); dbTester.commit(); return dto; } - private Organizations.UpdateWsResponse executeIdRequest(String uuid, @Nullable String name) { - return executeRequest(uuid, null, name, null, null, null); - } - - private Organizations.UpdateWsResponse executeIdRequest(String id, @Nullable String name, - @Nullable String description, @Nullable String url, @Nullable String avatar) { - return executeRequest(id, null, name, description, url, avatar); - } - private Organizations.UpdateWsResponse executeKeyRequest(String key, @Nullable String name) { - return executeRequest(null, key, name, null, null, null); + return executeRequest(key, name, null, null, null); } private Organizations.UpdateWsResponse executeKeyRequest(String key, @Nullable String name, @Nullable String description, @Nullable String url, @Nullable String avatar) { - return executeRequest(null, key, name, description, url, avatar); + return executeRequest(key, name, description, url, avatar); } - private Organizations.UpdateWsResponse executeRequest(@Nullable String id, @Nullable String key, + private Organizations.UpdateWsResponse executeRequest(@Nullable String key, @Nullable String name, @Nullable String description, @Nullable String url, @Nullable String avatar) { TestRequest request = wsTester.newRequest() .setMediaType(MediaTypes.PROTOBUF); - setParam(request, "id", id); setParam(request, "key", key); setParam(request, "name", name); setParam(request, "description", description); @@ -448,7 +296,6 @@ public class UpdateActionTest { @Nullable String description, @Nullable String url, @Nullable String avatar, long updateAt) { Organizations.Organization organization = response.getOrganization(); - assertThat(organization.getId()).isEqualTo(dto.getUuid()); assertThat(organization.getName()).isEqualTo(name); assertThat(organization.getKey()).isEqualTo(dto.getKey()); if (description == null) { diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/OrganizationService.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/OrganizationService.java index 9077250a242..df2e355e65b 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/OrganizationService.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/OrganizationService.java @@ -56,7 +56,6 @@ public class OrganizationService extends BaseService { public UpdateWsResponse update(UpdateWsRequest request) { PostRequest post = new PostRequest(path("update")) - .setParam("id", request.getId()) .setParam("key", request.getKey()) .setParam("name", request.getName()) .setParam("description", request.getDescription()) @@ -66,9 +65,8 @@ public class OrganizationService extends BaseService { return call(post, UpdateWsResponse.parser()); } - public void delete(@Nullable String id, @Nullable String key) { + public void delete(@Nullable String key) { PostRequest post = new PostRequest(path("delete")) - .setParam("id", id) .setParam("key", key); call(post); diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/UpdateWsRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/UpdateWsRequest.java index 2c131aa4d24..8167282d509 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/UpdateWsRequest.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/organization/UpdateWsRequest.java @@ -20,7 +20,6 @@ package org.sonarqube.ws.client.organization; public class UpdateWsRequest { - private final String id; private final String key; private final String name; private final String description; @@ -28,7 +27,6 @@ public class UpdateWsRequest { private final String avatar; public UpdateWsRequest(Builder builder) { - this.id = builder.id; this.name = builder.name; this.key = builder.key; this.description = builder.description; @@ -36,10 +34,6 @@ public class UpdateWsRequest { this.avatar = builder.avatar; } - public String getId() { - return id; - } - public String getName() { return name; } @@ -61,18 +55,12 @@ public class UpdateWsRequest { } public static class Builder { - private String id; private String key; private String name; private String description; private String url; private String avatar; - public Builder setId(String id) { - this.id = id; - return this; - } - public Builder setKey(String key) { this.key = key; return this; diff --git a/sonar-ws/src/main/protobuf/ws-organizations.proto b/sonar-ws/src/main/protobuf/ws-organizations.proto index 87754dd2142..4029ca50a75 100644 --- a/sonar-ws/src/main/protobuf/ws-organizations.proto +++ b/sonar-ws/src/main/protobuf/ws-organizations.proto @@ -40,10 +40,9 @@ message SearchWsResponse { } message Organization { - optional string id = 1; - optional string key = 2; - optional string name = 3; - optional string description = 4; - optional string url = 5; - optional string avatar = 6; + optional string key = 1; + optional string name = 2; + optional string description = 3; + optional string url = 4; + optional string avatar = 5; } |