Fix null collections in org.sonar.api.issue.IssueQuery

diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml
index 61246d6c239cdda5ca0cf12013a3d053cf88ebc4..1495d7bf359a4432d9fa59c02f01e07bf1670908 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml
@@ -230,32 +230,32 @@
     <if test="componentRootKeys != null and componentRootKeys.size() > 0">
       inner join (<include refid="org.sonar.core.resource.ResourceMapper.selectChildrenComponentIdsQuery" />) components on components.project_id=i.component_id
     </if>
-    <if test="query.components() != null and query.components().size() > 0">
+    <if test="query.components().size() > 0">
       inner join projects project_component on project_component.id=i.component_id and project_component.enabled=${_true} and project_component.kee in
       <foreach item="component" index="index" collection="query.components()" open="(" separator="," close=")">#{component}
       </foreach>
     </if>
-    <if test="query.rules() != null and query.rules().size() > 0">
+    <if test="query.rules().size()>0">
       inner join rules r on r.id=i.rule_id and (<foreach item="rule" index="index" collection="query.rules()" open="(" separator=" or " close=")">
       r.plugin_name=#{rule.repository} and r.plugin_rule_key=#{rule.rule}</foreach>)
     </if>
     <where>
-      <if test="query.issueKeys() != null">
+      <if test="query.issueKeys().size()>0">
         and i.kee in
         <foreach item="key" index="index" collection="query.issueKeys()" open="(" separator="," close=")">#{key}
         </foreach>
       </if>
-      <if test="query.severities() != null">
+      <if test="query.severities().size()>0">
         and i.severity in
         <foreach item="severity" index="index" collection="query.severities()" open="(" separator="," close=")">#{severity}
         </foreach>
       </if>
-      <if test="query.statuses() != null">
+      <if test="query.statuses().size()>0">
         and i.status in
         <foreach item="status" index="index" collection="query.statuses()" open="(" separator="," close=")">#{status}
         </foreach>
       </if>
-      <if test="query.resolutions() != null">
+      <if test="query.resolutions().size()>0">
         and i.resolution in
         <foreach item="resolution" index="index" collection="query.resolutions()" open="(" separator="," close=")">#{resolution}
         </foreach>
@@ -268,12 +268,12 @@
           and i.resolution is null
         </if>
       </if>
-      <if test="query.reporters() != null">
+      <if test="query.reporters().size()>0">
         and i.reporter in
         <foreach item="reporter" index="index" collection="query.reporters()" open="(" separator="," close=")">#{reporter}
         </foreach>
       </if>
-      <if test="query.assignees() != null">
+      <if test="query.assignees().size()>0">
         and i.assignee in
         <foreach item="assignee" index="index" collection="query.assignees()" open="(" separator="," close=")">#{assignee}
         </foreach>
@@ -294,7 +294,7 @@
           and i.action_plan_key is null
         </if>
       </if>
-      <if test="query.actionPlans() != null">
+      <if test="query.actionPlans().size()>0">
         and i.action_plan_key in
         <foreach item="action_plan" index="index" collection="query.actionPlans()" open="(" separator="," close=")">
           #{action_plan}
@@ -306,9 +306,6 @@
       <if test="query.createdBefore() != null">
         and i.issue_creation_date &lt; #{query.createdBefore}
       </if>
-      <if test="query.createdBefore() != null">
-        and i.issue_creation_date &lt; #{query.createdBefore}
-      </if>
     </where>
     order by i.id desc
   </sql>

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java
index 5daf278f46a21b4d49cb9ca1f660170989ac7e31..79e5aace148a48b3257136b3f8ceaf48d30dd4f6 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java
@@ -27,10 +27,7 @@ import org.sonar.api.web.UserRole;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-
-import java.util.Collection;
-import java.util.Date;
-import java.util.Set;
+import java.util.*;
 
 /**
  * @since 3.6
@@ -77,16 +74,16 @@ public class IssueQuery {
   private final int pageIndex;
 
   private IssueQuery(Builder builder) {
-    this.issueKeys = builder.issueKeys;
-    this.severities = builder.severities;
-    this.statuses = builder.statuses;
-    this.resolutions = builder.resolutions;
-    this.components = builder.components;
-    this.componentRoots = builder.componentRoots;
-    this.rules = builder.rules;
-    this.actionPlans = builder.actionPlans;
-    this.reporters = builder.reporters;
-    this.assignees = builder.assignees;
+    this.issueKeys = defaultCollection(builder.issueKeys);
+    this.severities = defaultCollection(builder.severities);
+    this.statuses = defaultCollection(builder.statuses);
+    this.resolutions = defaultCollection(builder.resolutions);
+    this.components = defaultCollection(builder.components);
+    this.componentRoots = defaultCollection(builder.componentRoots);
+    this.rules = defaultCollection(builder.rules);
+    this.actionPlans = defaultCollection(builder.actionPlans);
+    this.reporters = defaultCollection(builder.reporters);
+    this.assignees = defaultCollection(builder.assignees);
     this.assigned = builder.assigned;
     this.planned = builder.planned;
     this.resolved = builder.resolved;
@@ -224,52 +221,52 @@ public class IssueQuery {
     private Builder() {
     }
 
-    public Builder issueKeys(Collection<String> l) {
+    public Builder issueKeys(@Nullable Collection<String> l) {
       this.issueKeys = l;
       return this;
     }
 
-    public Builder severities(Collection<String> l) {
+    public Builder severities(@Nullable Collection<String> l) {
       this.severities = l;
       return this;
     }
 
-    public Builder statuses(Collection<String> l) {
+    public Builder statuses(@Nullable Collection<String> l) {
       this.statuses = l;
       return this;
     }
 
-    public Builder resolutions(Collection<String> l) {
+    public Builder resolutions(@Nullable Collection<String> l) {
       this.resolutions = l;
       return this;
     }
 
-    public Builder components(Collection<String> l) {
+    public Builder components(@Nullable Collection<String> l) {
       this.components = l;
       return this;
     }
 
-    public Builder componentRoots(Collection<String> l) {
+    public Builder componentRoots(@Nullable Collection<String> l) {
       this.componentRoots = l;
       return this;
     }
 
-    public Builder rules(Collection<RuleKey> rules) {
+    public Builder rules(@Nullable Collection<RuleKey> rules) {
       this.rules = rules;
       return this;
     }
 
-    public Builder actionPlans(Collection<String> l) {
+    public Builder actionPlans(@Nullable Collection<String> l) {
       this.actionPlans = l;
       return this;
     }
 
-    public Builder reporters(Collection<String> l) {
+    public Builder reporters(@Nullable Collection<String> l) {
       this.reporters = l;
       return this;
     }
 
-    public Builder assignees(Collection<String> l) {
+    public Builder assignees(@Nullable Collection<String> l) {
       this.assignees = l;
       return this;
     }
@@ -302,12 +299,12 @@ public class IssueQuery {
     }
 
     public Builder createdAfter(@Nullable Date d) {
-      this.createdAfter = d;
+      this.createdAfter = (d == null ? null : new Date(d.getTime()));
       return this;
     }
 
     public Builder createdBefore(@Nullable Date d) {
-      this.createdBefore = d;
+      this.createdBefore = (d == null ? null : new Date(d.getTime()));
       return this;
     }
 
@@ -367,4 +364,8 @@ public class IssueQuery {
       Preconditions.checkArgument(pageIndex > 0, "Page index must be greater than 0 (got " + pageIndex + ")");
     }
   }
+
+  private static <T> Collection<T> defaultCollection(@Nullable Collection<T> c) {
+    return c == null ? Collections.<T>emptyList() : Collections.unmodifiableCollection(c);
+  }
 }

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 3ee45572a6c19f7621bc48138426c1fe819d641b..27ee1da2ee86063938d050bf6f2b6ef559cdcb43 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
@@ -49,6 +49,8 @@ public class IssueQueryTest {
       .assigned(true)
       .createdAfter(new Date())
       .createdBefore(new Date())
+      .planned(true)
+      .resolved(true)
       .sort(IssueQuery.SORT_BY_ASSIGNEE)
       .pageSize(10)
       .pageIndex(2)
@@ -67,12 +69,66 @@ public class IssueQueryTest {
     assertThat(query.actionPlans()).containsOnly("AP1", "AP2");
     assertThat(query.createdAfter()).isNotNull();
     assertThat(query.createdBefore()).isNotNull();
+    assertThat(query.planned()).isTrue();
+    assertThat(query.resolved()).isTrue();
     assertThat(query.sort()).isEqualTo(IssueQuery.SORT_BY_ASSIGNEE);
     assertThat(query.pageSize()).isEqualTo(10);
     assertThat(query.pageIndex()).isEqualTo(2);
     assertThat(query.requiredRole()).isEqualTo(UserRole.USER);
   }
 
+  @Test
+  public void collection_params_should_not_be_null_but_empty() throws Exception {
+    IssueQuery query = IssueQuery.builder()
+      .issueKeys(null)
+      .components(null)
+      .componentRoots(null)
+      .statuses(null)
+      .actionPlans(null)
+      .assignees(null)
+      .reporters(null)
+      .resolutions(null)
+      .rules(null)
+      .severities(null)
+      .build();
+    assertThat(query.issueKeys()).isEmpty();
+    assertThat(query.components()).isEmpty();
+    assertThat(query.componentRoots()).isEmpty();
+    assertThat(query.statuses()).isEmpty();
+    assertThat(query.actionPlans()).isEmpty();
+    assertThat(query.assignees()).isEmpty();
+    assertThat(query.reporters()).isEmpty();
+    assertThat(query.resolutions()).isEmpty();
+    assertThat(query.rules()).isEmpty();
+    assertThat(query.severities()).isEmpty();
+  }
+
+  @Test
+  public void test_default_query() throws Exception {
+    IssueQuery query = IssueQuery.builder().build();
+    assertThat(query.issueKeys()).isEmpty();
+    assertThat(query.components()).isEmpty();
+    assertThat(query.componentRoots()).isEmpty();
+    assertThat(query.statuses()).isEmpty();
+    assertThat(query.actionPlans()).isEmpty();
+    assertThat(query.assignees()).isEmpty();
+    assertThat(query.reporters()).isEmpty();
+    assertThat(query.resolutions()).isEmpty();
+    assertThat(query.rules()).isEmpty();
+    assertThat(query.severities()).isEmpty();
+    assertThat(query.assigned()).isNull();
+    assertThat(query.createdAfter()).isNull();
+    assertThat(query.createdBefore()).isNull();
+    assertThat(query.planned()).isNull();
+    assertThat(query.resolved()).isNull();
+    assertThat(query.sort()).isNull();
+    assertThat(query.pageSize()).isEqualTo(100);
+    assertThat(query.pageIndex()).isEqualTo(1);
+    assertThat(query.requiredRole()).isEqualTo(UserRole.USER);
+    assertThat(query.maxResults()).isEqualTo(IssueQuery.MAX_RESULTS);
+
+  }
+
   @Test
   public void should_use_max_page_size_if_negative() throws Exception {
     IssueQuery query = IssueQuery.builder().pageSize(0).build();