From: Julien Lancelot <julien.lancelot@sonarsource.com>
Date: Wed, 1 Feb 2017 09:19:17 +0000 (+0100)
Subject: SONAR-7300 Do not generate IllegalArgumentException when id is missing
X-Git-Tag: 6.3-RC1~237
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b81bbbdafa5aec8144fbf45fffa7f8df033ef06a;p=sonarqube.git

SONAR-7300 Do not generate IllegalArgumentException when id is missing
---

diff --git a/it/it-tests/src/test/java/it/settings/DeprecatedPropertiesWsTest.java b/it/it-tests/src/test/java/it/settings/DeprecatedPropertiesWsTest.java
index 876cfccd071..34c438be489 100644
--- a/it/it-tests/src/test/java/it/settings/DeprecatedPropertiesWsTest.java
+++ b/it/it-tests/src/test/java/it/settings/DeprecatedPropertiesWsTest.java
@@ -273,6 +273,12 @@ public class DeprecatedPropertiesWsTest {
     assertThat(setting.getValues()).containsOnly("value1", "value2", "value3");
   }
 
+  @Test
+  public void fail_with_error_400_when_put_property_without_id() throws Exception {
+    Response response = putProperty("", "some-value", null, false);
+    assertThat(response.code()).isEqualTo(400);
+  }
+
   @Test
   public void delete_property() throws Exception {
     setProperty("custom-property", "value", null);
@@ -335,20 +341,20 @@ public class DeprecatedPropertiesWsTest {
     return Arrays.stream(properties).findFirst().orElseGet(() -> null);
   }
 
-  private static void putProperty(String key, String value, @Nullable String componentKey, boolean useIdParameter) throws UnsupportedEncodingException {
+  private static Response putProperty(String key, String value, @Nullable String componentKey, boolean useIdParameter) throws UnsupportedEncodingException {
     String url = useIdParameter ? orchestrator.getServer().getUrl() + "/api/properties?id=" + encode(key, "UTF-8") + "&value=" + value
       : orchestrator.getServer().getUrl() + "/api/properties/" + encode(key, "UTF-8") + "?value=" + value;
     url += componentKey != null ? "&resource=" + componentKey : "";
-    call(new Request.Builder()
+    return call(new Request.Builder()
       .put(new FormBody.Builder().build())
       .url(url));
   }
 
-  private static void deleteProperty(String key, @Nullable String componentKey, boolean useIdParameter) throws UnsupportedEncodingException {
+  private static Response deleteProperty(String key, @Nullable String componentKey, boolean useIdParameter) throws UnsupportedEncodingException {
     String url = useIdParameter ? orchestrator.getServer().getUrl() + "/api/properties?id=" + encode(key, "UTF-8")
       : orchestrator.getServer().getUrl() + "/api/properties/" + encode(key, "UTF-8");
     url += componentKey != null ? "?resource=" + componentKey : "";
-    call(new Request.Builder()
+    return call(new Request.Builder()
       .delete(new FormBody.Builder().build())
       .url(url));
   }
@@ -356,14 +362,12 @@ public class DeprecatedPropertiesWsTest {
   private static Response call(Request.Builder requestBuilder) {
     try {
       requestBuilder.header("Authorization", Credentials.basic("admin", "admin"));
-      Response response = new OkHttpClient.Builder()
+      return new OkHttpClient.Builder()
         .connectTimeout(30, TimeUnit.SECONDS)
         .readTimeout(30, TimeUnit.SECONDS)
         .build()
         .newCall(requestBuilder.build())
         .execute();
-      assertThat(response.isSuccessful()).as("Error code is '%s', error message is '%s'", Integer.toString(response.code()), response.body().string()).isTrue();
-      return response;
     } catch (IOException e) {
       throw Throwables.propagate(e);
     }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/DeprecatedPropertiesWsFilter.java b/server/sonar-server/src/main/java/org/sonar/server/ws/DeprecatedPropertiesWsFilter.java
index b940440d018..cd4c8727655 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/ws/DeprecatedPropertiesWsFilter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ws/DeprecatedPropertiesWsFilter.java
@@ -40,7 +40,6 @@ import org.apache.commons.io.IOUtils;
 import org.sonar.api.web.ServletFilter;
 import org.sonar.server.property.ws.IndexAction;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.sonar.server.property.ws.PropertiesWs.CONTROLLER_PROPERTIES;
@@ -147,11 +146,9 @@ public class DeprecatedPropertiesWsFilter extends ServletFilter {
       switch (method) {
         case "POST":
         case "PUT":
-          failIfIdIsNull(id);
           handlePutAndPost(id, getValues(), getComponent());
           break;
         case "DELETE":
-          failIfIdIsNull(id);
           handleDelete(id, getComponent());
           break;
         default:
@@ -248,9 +245,6 @@ public class DeprecatedPropertiesWsFilter extends ServletFilter {
       value.ifPresent(s -> additionalParams.put(parameterKey, s));
     }
 
-    private static void failIfIdIsNull(Optional<String> id) {
-      checkArgument(id.isPresent(), "The 'id' parameter is missing");
-    }
   }
 
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/DeprecatedPropertiesWsFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/DeprecatedPropertiesWsFilterTest.java
index 370ac221726..d139c3ed6e9 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/ws/DeprecatedPropertiesWsFilterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/ws/DeprecatedPropertiesWsFilterTest.java
@@ -264,36 +264,6 @@ public class DeprecatedPropertiesWsFilterTest {
     assertNoParam("value", "values");
   }
 
-  @Test
-  public void fail_to_redirect_put_api_properties_when_no_id() throws Exception {
-    when(request.getRequestURI()).thenReturn("/api/properties/");
-    when(request.getMethod()).thenReturn("PUT");
-
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("The 'id' parameter is missing");
-    underTest.doFilter(request, response, chain);
-  }
-
-  @Test
-  public void fail_to_redirect_post_api_properties_when_no_id() throws Exception {
-    when(request.getRequestURI()).thenReturn("/api/properties/");
-    when(request.getMethod()).thenReturn("POST");
-
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("The 'id' parameter is missing");
-    underTest.doFilter(request, response, chain);
-  }
-
-  @Test
-  public void fail_to_redirect_delete_api_properties_when_no_id() throws Exception {
-    when(request.getRequestURI()).thenReturn("/api/properties");
-    when(request.getMethod()).thenReturn("DELETE");
-
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("The 'id' parameter is missing");
-    underTest.doFilter(request, response, chain);
-  }
-
   private void assertRedirection(String path, String method) {
     verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
     assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo(path);