SONAR-8172 make all args of api/organizations/update optional but key

all but key

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 5187d7a76ae0db1dba4f7f0ff34d4960fce3781f..d2a0d3f25a2543ed26c59caee80f9342cf666b93 100644 (file)
--- 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 f7d81eaf234a701621bc5ab59c1015c141e8fa3a..b94dc7ab60b59f92689b3cac99ccd2b9dd5912d4 100644 (file)
--- 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 38b3a96d71907c4ccca615490d64d87cfdbdd905..75e0f293c5d9a8fb68abb1fb25fac88181a93b35 100644 (file)
--- 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. <br />" +
         "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 64e8be3503e98be69c22b652c631a5d11f1a27cc..f9ff82536ac97889393f698f51f4b390b5249960 100644 (file)
--- 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> 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<String> name;
+    private final Request.Param<String> description;
+    private final Request.Param<String> url;
+    private final Request.Param<String> avatar;
+
+    private UpdateOrganizationRequest(Request.Param<String> name,
+      Request.Param<String> description,
+      Request.Param<String> url,
+      Request.Param<String> avatar) {
+      this.name = name;
+      this.description = description;
+      this.url = url;
+      this.avatar = avatar;
+    }
+
+    public Request.Param<String> getName() {
+      return name;
+    }
+
+    public Request.Param<String> getDescription() {
+      return description;
+    }
+
+    public Request.Param<String> getUrl() {
+      return url;
+    }
+
+    public Request.Param<String> 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 188b54f0049a796db399186387ea849c9e3af85d..e345ae6d5b224f075a1f93aabacee50a5cd064d7 100644 (file)
--- 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,