SONAR-4421 Remove empty actions and issues from bulk change parameters

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java
index feb57e3c984bc06092b8898e6c370cbf31df0d45..adac4a108bd269362c9e7079029aca4fbd15dd9a 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.api.issue;
 
-import com.google.common.collect.Lists;
 import org.junit.Test;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
@@ -27,7 +26,9 @@ import org.sonar.api.web.UserRole;
 
 import java.util.Arrays;
 import java.util.Date;
+import java.util.List;
 
+import static com.google.common.collect.Lists.newArrayList;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
 
@@ -36,16 +37,16 @@ public class IssueQueryTest {
   @Test
   public void should_build_query() throws Exception {
     IssueQuery query = IssueQuery.builder()
-      .issueKeys(Lists.newArrayList("ABCDE"))
-      .severities(Lists.newArrayList(Severity.BLOCKER))
-      .statuses(Lists.newArrayList(Issue.STATUS_RESOLVED))
-      .resolutions(Lists.newArrayList(Issue.RESOLUTION_FALSE_POSITIVE))
-      .components(Lists.newArrayList("org/struts/Action.java"))
-      .componentRoots(Lists.newArrayList("org.struts:core"))
-      .rules(Lists.newArrayList(RuleKey.of("squid", "AvoidCycle")))
-      .actionPlans(Lists.newArrayList("AP1", "AP2"))
-      .reporters(Lists.newArrayList("crunky"))
-      .assignees(Lists.newArrayList("gargantua"))
+      .issueKeys(newArrayList("ABCDE"))
+      .severities(newArrayList(Severity.BLOCKER))
+      .statuses(newArrayList(Issue.STATUS_RESOLVED))
+      .resolutions(newArrayList(Issue.RESOLUTION_FALSE_POSITIVE))
+      .components(newArrayList("org/struts/Action.java"))
+      .componentRoots(newArrayList("org.struts:core"))
+      .rules(newArrayList(RuleKey.of("squid", "AvoidCycle")))
+      .actionPlans(newArrayList("AP1", "AP2"))
+      .reporters(newArrayList("crunky"))
+      .assignees(newArrayList("gargantua"))
       .assigned(true)
       .createdAfter(new Date())
       .createdBefore(new Date())
@@ -82,7 +83,7 @@ public class IssueQueryTest {
   @Test
   public void should_build_query_without_dates() throws Exception {
     IssueQuery query = IssueQuery.builder()
-      .issueKeys(Lists.newArrayList("ABCDE"))
+      .issueKeys(newArrayList("ABCDE"))
       .build();
 
     assertThat(query.issueKeys()).containsOnly("ABCDE");
@@ -195,6 +196,22 @@ public class IssueQueryTest {
     }
   }
 
+  @Test
+  public void number_of_issue_keys_should_be_limited() throws Exception {
+    List<String> issueKeys = newArrayList();
+    for (int i=0; i<IssueQuery.MAX_ISSUE_KEYS; i++) {
+      issueKeys.add("issue-key-"+ i);
+    }
+    try {
+      IssueQuery.builder()
+          .issueKeys(issueKeys)
+          .build();
+      fail();
+    } catch (Exception e) {
+      assertThat(e).hasMessage("Number of issue keys must be less than 500 (got 500)").isInstanceOf(IllegalArgumentException.class);
+    }
+  }
+
   @Test
   public void should_accept_null_sort() throws Exception {
     IssueQuery query = IssueQuery.builder().sort(null).build();

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
index be096d2b15e485bf81b96d610ffb23fb43cf7fe6..4654e23ae2fc139d446b564db58be20442036a28 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
@@ -21,15 +21,20 @@
 package org.sonar.server.issue;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.ReflectionToStringBuilder;
 import org.sonar.server.util.RubyUtils;
 
+import javax.annotation.Nullable;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 
 /**
@@ -52,11 +57,11 @@ public class IssueBulkChangeQuery {
   }
 
   private void parse(Map<String, Object> props, String comment) {
-    this.issues = RubyUtils.toStrings(props.get("issues"));
+    this.issues = sanitizeList(RubyUtils.toStrings(props.get("issues")));
     if (issues == null || issues.isEmpty()) {
       throw new IllegalArgumentException("Issues must not be empty");
     }
-    actions = RubyUtils.toStrings(props.get("actions"));
+    actions = sanitizeList(RubyUtils.toStrings(props.get("actions")));
     if (actions == null || actions.isEmpty()) {
       throw new IllegalArgumentException("At least one action must be provided");
     }
@@ -72,6 +77,18 @@ public class IssueBulkChangeQuery {
     }
   }
 
+  private List<String> sanitizeList(List<String> list){
+    if (list == null || list.isEmpty()){
+      return Collections.emptyList();
+    }
+    return newArrayList(Iterables.filter(list, new Predicate<String>() {
+      @Override
+      public boolean apply(@Nullable String input) {
+        return !Strings.isNullOrEmpty(input);
+      }
+    }));
+  }
+
   public List<String> issues() {
     return issues;
   }

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb
index 0baa99b8a03b5b08570963ac211e89165e577bf5..0fcfa5b1c41c782b1c81bfc24313d4cf3d2fc716 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb
@@ -1,7 +1,7 @@
 <form id="bulk-change-form" method="post" action="<%= ApplicationController.root_context -%>/issues/bulk_change">
   <input type="hidden" name="issues" value="<%= @issues.join(',') -%>">
   <input type="hidden" name="criteria_params" value="<%= @criteria_params.to_query -%>">
-  <input type="hidden" name="actions[]" id="bulk-change-transition-action" value="">
+  <input type="hidden" name="actions[]" id="bulk-change-transition-action">
   <fieldset>
     <div class="modal-head">
       <h2><%= message('issue_filter.bulk_change.form.title', {:params => @issues.size.to_s}) -%></h2>

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
index 08cf08e1216643ad9c1ba270abf28945dcf3e44b..4fa46bf44b7282757ba572ff313f9158cd54fde4 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
@@ -47,6 +47,30 @@ public class IssueBulkChangeQueryTest {
     assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH");
   }
 
+  @Test
+  public void should_remove_empty_actions() {
+    Map<String, Object> params = newHashMap();
+    params.put("issues", newArrayList("ABCD", "EFGH"));
+    params.put("actions", newArrayList("do_transition", "", null));
+    params.put("do_transition.transition", "confirm");
+
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params);
+    assertThat(issueBulkChangeQuery.actions()).containsOnly("do_transition");
+    assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH");
+  }
+
+  @Test
+  public void should_remove_empty_issues() {
+    Map<String, Object> params = newHashMap();
+    params.put("issues", newArrayList("ABCD", "EFGH", "", null));
+    params.put("actions", newArrayList("do_transition"));
+    params.put("do_transition.transition", "confirm");
+
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params);
+    assertThat(issueBulkChangeQuery.actions()).containsOnly("do_transition");
+    assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH");
+  }
+
   @Test
   public void should_create_query_with_comment() {
     Map<String, Object> params = newHashMap();