From 7941a52aeb459349a6baedc7de780edcc1d0f295 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 7 Nov 2016 17:07:04 +0100 Subject: [PATCH] SONAR-8172 make all args of api/organizations/update optional but key all but key --- .../java/it/organization/OrganizationIt.java | 3 + .../server/organization/ws/CreateAction.java | 4 +- .../ws/OrganizationsWsSupport.java | 21 ++++-- .../server/organization/ws/UpdateAction.java | 68 ++++++++++++++++--- .../organization/ws/UpdateActionTest.java | 31 +++++---- 5 files changed, 100 insertions(+), 27 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 5187d7a76ae..d2a0d3f25a2 100644 --- a/it/it-tests/src/test/java/it/organization/OrganizationIt.java +++ b/it/it-tests/src/test/java/it/organization/OrganizationIt.java @@ -111,6 +111,9 @@ public class OrganizationIt { adminOrganizationService.update(new UpdateWsRequest.Builder() .setKey(createdOrganization.getKey()) .setName("new name 3") + .setDescription("") + .setUrl("") + .setAvatar("") .build()); verifySingleSearchResult(createdOrganization, "new name 3", null, null, null); 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 f7d81eaf234..b94dc7ab60b 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 @@ -87,7 +87,7 @@ public class CreateAction implements OrganizationsAction { "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)") .setExampleValue("foo-company"); - wsSupport.addOrganizationDetailsParams(action); + wsSupport.addOrganizationDetailsParams(action, true); } @Override @@ -98,7 +98,7 @@ public class CreateAction implements OrganizationsAction { userSession.checkIsRoot(); } - String name = wsSupport.getAndCheckName(request); + String name = wsSupport.getAndCheckMandatoryName(request); String requestKey = getAndCheckKey(request); String key = useOrGenerateKey(requestKey, name); wsSupport.getAndCheckDescription(request); 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 38b3a96d719..75e0f293c5d 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 @@ -43,11 +43,24 @@ public class OrganizationsWsSupport { static final int DESCRIPTION_MAX_LENGTH = 256; static final int URL_MAX_LENGTH = 256; - String getAndCheckName(Request request) { + String getAndCheckMandatoryName(Request request) { String name = request.mandatoryParam(PARAM_NAME); + checkName(name); + return name; + } + + @CheckForNull + String getAndCheckName(Request request) { + String name = request.param(PARAM_NAME); + if (name != null) { + checkName(name); + } + return name; + } + + private static void checkName(String name) { checkArgument(name.length() >= NAME_MIN_LENGTH, "Name '%s' must be at least %s chars long", name, NAME_MIN_LENGTH); checkArgument(name.length() <= NAME_MAX_LENGTH, "Name '%s' must be at most %s chars long", name, NAME_MAX_LENGTH); - return name; } @CheckForNull @@ -74,9 +87,9 @@ public class OrganizationsWsSupport { return value; } - void addOrganizationDetailsParams(WebService.NewAction action) { + void addOrganizationDetailsParams(WebService.NewAction action, boolean isNameRequired) { action.createParam(PARAM_NAME) - .setRequired(true) + .setRequired(isNameRequired) .setDescription("Name of the organization.
" + "It must be between 2 and 64 chars longs.") .setExampleValue("Foo Company"); 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 64e8be3503e..f9ff82536ac 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,6 +20,8 @@ package org.sonar.server.organization.ws; import java.util.Optional; +import javax.annotation.CheckForNull; +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; @@ -32,7 +34,11 @@ import org.sonarqube.ws.Organizations; import static java.lang.String.format; import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN; +import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_AVATAR_URL; +import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_DESCRIPTION; 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.OrganizationsWsSupport.PARAM_URL; import static org.sonar.server.ws.WsUtils.writeProtobuf; public class UpdateAction implements OrganizationsAction { @@ -63,7 +69,7 @@ public class UpdateAction implements OrganizationsAction { .setDescription("Organization key") .setExampleValue("foo-company"); - wsSupport.addOrganizationDetailsParams(action); + wsSupport.addOrganizationDetailsParams(action, false); } @Override @@ -71,20 +77,22 @@ public class UpdateAction implements OrganizationsAction { userSession.checkLoggedIn(); 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); + + UpdateOrganizationRequest updateRequest = new UpdateOrganizationRequest( + request.getParam(PARAM_NAME, (rqt, paramKey) -> wsSupport.getAndCheckName(rqt)), + request.getParam(PARAM_DESCRIPTION, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckDescription(rqt))), + request.getParam(PARAM_URL, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckUrl(rqt))), + request.getParam(PARAM_AVATAR_URL, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckAvatar(rqt)))); try (DbSession dbSession = dbClient.openSession(false)) { OrganizationDto dto = getDto(dbSession, key); userSession.checkOrganizationPermission(dto.getUuid(), SYSTEM_ADMIN); - dto.setName(name) - .setDescription(description) - .setUrl(url) - .setAvatarUrl(avatar); + dto.setName(updateRequest.getName().or(dto::getName)) + .setDescription(updateRequest.getDescription().or(dto::getDescription)) + .setUrl(updateRequest.getUrl().or(dto::getUrl)) + .setAvatarUrl(updateRequest.getAvatar().or(dto::getAvatarUrl)); dbClient.organizationDao().update(dbSession, dto); dbSession.commit(); @@ -92,6 +100,14 @@ public class UpdateAction implements OrganizationsAction { } } + @CheckForNull + private static String emptyAsNull(@Nullable String value) { + if (value == null || value.isEmpty()) { + return null; + } + return value; + } + private OrganizationDto getDto(DbSession dbSession, String key) { Optional organizationDto = dbClient.organizationDao().selectByKey(dbSession, key); if (!organizationDto.isPresent()) { @@ -106,4 +122,38 @@ public class UpdateAction implements OrganizationsAction { request, response); } + + private static final class UpdateOrganizationRequest { + private final Request.Param name; + private final Request.Param description; + private final Request.Param url; + private final Request.Param avatar; + + private UpdateOrganizationRequest(Request.Param name, + Request.Param description, + Request.Param url, + Request.Param avatar) { + this.name = name; + this.description = description; + this.url = url; + this.avatar = avatar; + } + + public Request.Param getName() { + return name; + } + + public Request.Param getDescription() { + return description; + } + + public Request.Param getUrl() { + return url; + } + + public Request.Param getAvatar() { + return avatar; + } + } + } 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 188b54f0049..e345ae6d5b2 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 @@ -79,7 +79,7 @@ public class UpdateActionTest { .matches(param -> "foo-company".equals(param.exampleValue())) .matches(param -> "Organization key".equals(param.description())); assertThat(action.param("name")) - .matches(WebService.Param::isRequired) + .matches(param -> !param.isRequired()) .matches(param -> "Foo Company".equals(param.exampleValue())) .matches(param -> param.description() != null); assertThat(action.param("description")) @@ -139,13 +139,11 @@ public class UpdateActionTest { } @Test - public void request_fails_if_name_param_is_missing() { - userSession.login(); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("The 'name' parameter is missing"); + public void request_with_only_key_param_does_not_fail_and_updates_only_updateAt_field() { + OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); + giveUserSystemAdminPermission(dto); - executeKeyRequest("key", null); + verifyResponseAndDb(executeKeyRequest(dto.getKey(), null), dto, dto.getName(), DATE_2); } @Test @@ -192,7 +190,7 @@ public class UpdateActionTest { giveUserSystemAdminPermission(dto); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, null, null); - verifyResponseAndDb(response, dto, "bar", null, null, null, DATE_2); + verifyResponseAndDb(response, dto, "bar", DATE_2); } @Test @@ -221,7 +219,7 @@ public class UpdateActionTest { String description = STRING_257_CHARS_LONG.substring(0, 256); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", description, null, null); - verifyResponseAndDb(response, dto, "bar", description, null, null, DATE_2); + verifyResponseAndDb(response, dto, "bar", description, dto.getUrl(), dto.getAvatarUrl(), DATE_2); } @Test @@ -241,7 +239,7 @@ public class UpdateActionTest { giveUserSystemAdminPermission(dto); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, url, null); - verifyResponseAndDb(response, dto, "bar", null, url, null, DATE_2); + verifyResponseAndDb(response, dto, "bar", dto.getDescription(), url, dto.getAvatarUrl(), DATE_2); } @Test @@ -261,7 +259,16 @@ public class UpdateActionTest { String avatar = STRING_257_CHARS_LONG.substring(0, 256); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, null, avatar); - verifyResponseAndDb(response, dto, "bar", null, null, avatar, DATE_2); + verifyResponseAndDb(response, dto, "bar", dto.getDescription(), dto.getUrl(), avatar, DATE_2); + } + + @Test + public void request_removes_optional_parameters_when_associated_parameter_are_empty() { + OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); + giveUserSystemAdminPermission(dto); + + Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bla", "", "", ""); + verifyResponseAndDb(response, dto, "bla", null, null, null, DATE_2); } private void giveUserSystemAdminPermission(OrganizationDto organizationDto) { @@ -307,7 +314,7 @@ public class UpdateActionTest { } private void verifyResponseAndDb(Organizations.UpdateWsResponse response, OrganizationDto dto, String name, long updateAt) { - verifyResponseAndDb(response, dto, name, null, null, null, updateAt); + verifyResponseAndDb(response, dto, name, dto.getDescription(), dto.getUrl(), dto.getAvatarUrl(), updateAt); } private void verifyResponseAndDb(Organizations.UpdateWsResponse response, -- 2.39.5