From c433a8f51300a0d5cf8b5c322555fe73c677ce8c Mon Sep 17 00:00:00 2001
From: Julien Lancelot <julien.lancelot@gmail.com>
Date: Mon, 1 Jul 2013 11:37:55 +0200
Subject: [PATCH] SONAR-4461 Displaying more than 2000 issues in the Issues
 code viewer fail on SQL Server

---
 .../sonar/core/issue/db/ActionPlanDao.java    |  7 +++++-
 .../sonar/core/issue/db/ActionPlanMapper.java |  2 +-
 .../sonar/core/issue/db/IssueChangeDao.java   |  8 +++++--
 .../core/issue/db/IssueChangeMapper.java      |  2 +-
 .../org/sonar/core/issue/db/IssueDao.java     | 23 ++++++++-----------
 .../org/sonar/core/resource/ResourceDao.java  |  8 ++++++-
 .../sonar/core/resource/ResourceMapper.java   |  2 +-
 .../sonar/core/issue/db/ActionPlanMapper.xml  |  7 +++---
 .../sonar/core/issue/db/IssueChangeMapper.xml | 10 ++++----
 .../org/sonar/core/issue/db/IssueMapper.xml   |  8 +++----
 .../sonar/core/resource/ResourceMapper.xml    |  9 ++++----
 .../core/issue/db/ActionPlanDaoTest.java      | 19 +++++++++++++++
 .../core/issue/db/IssueChangeDaoTest.java     |  2 +-
 .../org/sonar/core/issue/db/IssueDaoTest.java |  2 +-
 .../sonar/core/resource/ResourceDaoTest.java  | 14 +++++++++++
 15 files changed, 81 insertions(+), 42 deletions(-)

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanDao.java
index 5e00190c152..1cd9ca14c57 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanDao.java
@@ -88,8 +88,13 @@ public class ActionPlanDao implements BatchComponent, ServerComponent {
     }
     SqlSession session = mybatis.openSession();
     try {
+      List<ActionPlanDto> dtosList = newArrayList();
       List<List<String>> keysPartition = Lists.partition(newArrayList(keys), 1000);
-      return session.getMapper(ActionPlanMapper.class).findByKeys(keysPartition);
+      for (List<String> partition : keysPartition) {
+        List<ActionPlanDto> dtos = session.getMapper(ActionPlanMapper.class).findByKeys(partition);
+        dtosList.addAll(dtos);
+      }
+      return dtosList;      
     } finally {
       MyBatis.closeQuietly(session);
     }
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanMapper.java
index 4ea956c9a9a..7af6718c9a4 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanMapper.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/db/ActionPlanMapper.java
@@ -35,7 +35,7 @@ public interface ActionPlanMapper {
 
   void delete(@Param("key") String key);
 
-  List<ActionPlanDto> findByKeys(@Param("keys") List<List<String>> keys);
+  List<ActionPlanDto> findByKeys(@Param("keys") List<String> keys);
 
   ActionPlanDto findByKey(@Param("key") String key);
 
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java
index cf9e5c6a678..8b5087caccd 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java
@@ -87,9 +87,13 @@ public class IssueChangeDao implements BatchComponent, ServerComponent {
       return Collections.emptyList();
     }
     IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class);
-
+    List<IssueChangeDto> dtosList = newArrayList();
     List<List<String>> keysPartition = Lists.partition(newArrayList(issueKeys), 1000);
-    return mapper.selectByIssuesAndType(keysPartition, changeType);
+    for (List<String> partition : keysPartition) {
+      List<IssueChangeDto> dtos = mapper.selectByIssuesAndType(partition, changeType);
+      dtosList.addAll(dtos);
+    }
+    return dtosList;
   }
 
   public boolean delete(String key) {
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java
index edb57a03bcd..91a05b8854c 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java
@@ -43,7 +43,7 @@ public interface IssueChangeMapper {
   /**
    * Issue changes by chronological date of creation
    */
-  List<IssueChangeDto> selectByIssuesAndType(@Param("issueKeys") List<List<String>> issueKeys,
+  List<IssueChangeDto> selectByIssuesAndType(@Param("issueKeys") List<String> issueKeys,
                                              @Param("changeType") String changeType);
 
   List<IssueChangeDto> selectByIssue(String issueKey);
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java
index 1fa8b6d024c..928e84317fd 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java
@@ -35,10 +35,8 @@ import javax.annotation.Nullable;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
 
 import static com.google.common.collect.Lists.newArrayList;
-import static com.google.common.collect.Maps.newHashMap;
 
 /**
  * @since 3.6
@@ -101,25 +99,21 @@ public class IssueDao implements BatchComponent, ServerComponent {
 
   private List<IssueDto> selectIssueIds(IssueQuery query, @Nullable Integer userId, Integer maxResults, SqlSession session){
     IssueMapper mapper = session.getMapper(IssueMapper.class);
-    return mapper.selectIssues(query, query.componentRoots(), userId, query.requiredRole(), maxResults, true);
+    return mapper.selectIssueIds(query, query.componentRoots(), userId, query.requiredRole(), maxResults);
   }
 
   public List<IssueDto> selectIssues(IssueQuery query) {
     SqlSession session = mybatis.openSession();
     try {
-      return selectIssues(query, null, Integer.MAX_VALUE, session);
+      return selectIssues(query, null, session);
     } finally {
       MyBatis.closeQuietly(session);
     }
   }
 
   public List<IssueDto> selectIssues(IssueQuery query, @Nullable Integer userId, SqlSession session){
-    return selectIssues(query, userId, query.maxResults(), session);
-  }
-
-  public List<IssueDto> selectIssues(IssueQuery query, @Nullable Integer userId, Integer maxResults, SqlSession session){
     IssueMapper mapper = session.getMapper(IssueMapper.class);
-    return mapper.selectIssues(query, query.componentRoots(), userId, query.requiredRole(), maxResults, false);
+    return mapper.selectIssues(query, query.componentRoots(), userId, query.requiredRole());
   }
 
   @VisibleForTesting
@@ -136,9 +130,12 @@ public class IssueDao implements BatchComponent, ServerComponent {
     if (ids.isEmpty()) {
       return Collections.emptyList();
     }
-    Object idsPartition = Lists.partition(newArrayList(ids), 1000);
-    Map<String, Object> params = newHashMap();
-    params.put("ids", idsPartition);
-    return session.selectList("org.sonar.core.issue.db.IssueMapper.selectByIds", params);
+    List<IssueDto> dtosList = newArrayList();
+    List<List<Long>> idsPartitionList = Lists.partition(newArrayList(ids), 1000);
+    for (List<Long> idsPartition : idsPartitionList) {
+      List<IssueDto> dtos = session.selectList("org.sonar.core.issue.db.IssueMapper.selectByIds", newArrayList(idsPartition));
+      dtosList.addAll(dtos);
+    }
+    return dtosList;
   }
 }
diff --git a/sonar-core/src/main/java/org/sonar/core/resource/ResourceDao.java b/sonar-core/src/main/java/org/sonar/core/resource/ResourceDao.java
index bcb7514322d..09c7e993a4f 100644
--- a/sonar-core/src/main/java/org/sonar/core/resource/ResourceDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/resource/ResourceDao.java
@@ -143,8 +143,14 @@ public class ResourceDao {
     }
     SqlSession session = mybatis.openSession();
     try {
+
+      List<ResourceDto> resources = newArrayList();
       List <List<Long>> idsPartition = Lists.partition(newArrayList(ids), 1000);
-      Collection<ResourceDto> resources =  session.getMapper(ResourceMapper.class).selectResourcesById(idsPartition);
+      for (List<Long> partition : idsPartition) {
+        List<ResourceDto> dtos = session.getMapper(ResourceMapper.class).selectResourcesById(partition);
+        resources.addAll(dtos);
+      }
+
       Collection<Component> components = newArrayList();
       for (ResourceDto resourceDto : resources) {
         components.add(toComponent(resourceDto));
diff --git a/sonar-core/src/main/java/org/sonar/core/resource/ResourceMapper.java b/sonar-core/src/main/java/org/sonar/core/resource/ResourceMapper.java
index ea89cbfac58..2632a0d1929 100644
--- a/sonar-core/src/main/java/org/sonar/core/resource/ResourceMapper.java
+++ b/sonar-core/src/main/java/org/sonar/core/resource/ResourceMapper.java
@@ -56,7 +56,7 @@ public interface ResourceMapper {
   /**
    * @since3.6
    */
-  List<ResourceDto> selectResourcesById(@Param("ids") List <List<Long>> ids);
+  List<ResourceDto> selectResourcesById(@Param("ids") List<Long> ids);
 
   /**
    * @since 3.6
diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/ActionPlanMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/ActionPlanMapper.xml
index 663284e21ac..590b3147086 100644
--- a/sonar-core/src/main/resources/org/sonar/core/issue/db/ActionPlanMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/ActionPlanMapper.xml
@@ -61,10 +61,9 @@
     select <include refid="actionPlanColumns"/>
     from action_plans ap, projects p
     <where>
-      <foreach collection="keys" open="ap.kee in (" close=")" item="list" separator=") or ap.kee in (" >
-        <foreach collection="list" item="element" separator=",">
-          #{element}
-        </foreach>
+      and ap.kee in
+      <foreach collection="keys" open="(" close=")" item="key" separator=",">
+        #{key}
       </foreach>
       and ap.project_id=p.id
     </where>
diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml
index d9d778458a0..7afcc6e1ed4 100644
--- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml
@@ -42,12 +42,10 @@
     select
     <include refid="issueChangeColumns"/>
     from issue_changes c
-    where c.change_type=#{changeType} and (
-    <foreach collection="issueKeys" open="c.issue_key in (" close=")" item="list" separator=") or c.issue_key in (">
-      <foreach collection="list" item="element" separator=",">
-        #{element}
-      </foreach>
-    </foreach>)
+    where c.change_type=#{changeType} and c.issue_key in
+    <foreach collection="issueKeys" open="(" close=")" item="key" separator=",">
+      #{key}
+    </foreach>
     order by c.created_at
   </select>
 
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 f407ec3289a..fdd87b94a1e 100644
--- 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
@@ -192,11 +192,9 @@
     inner join projects p on p.id=i.component_id
     inner join projects root on root.id=i.root_component_id
     <where>
-      and
-      <foreach collection="ids" open="i.id in (" close=")" item="list" separator=") or i.id in (">
-        <foreach collection="list" item="element" separator=",">
-          #{element}
-        </foreach>
+      and i.id in
+      <foreach collection="list" open="(" close=")" item="id" separator=",">
+        #{id}
       </foreach>
     </where>
   </select>
diff --git a/sonar-core/src/main/resources/org/sonar/core/resource/ResourceMapper.xml b/sonar-core/src/main/resources/org/sonar/core/resource/ResourceMapper.xml
index 394c9d56b73..c376df909d4 100644
--- a/sonar-core/src/main/resources/org/sonar/core/resource/ResourceMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/resource/ResourceMapper.xml
@@ -76,11 +76,10 @@
   </select>
 
   <select id="selectResourcesById" parameterType="map" resultMap="resourceResultMap">
-    select * from projects p where p.enabled=${_true} and
-    <foreach collection="ids" open="p.id in (" close=")" item="list" separator=") or p.id in (" >
-      <foreach collection="list" item="element" separator=",">
-        #{element}
-      </foreach>
+    select * from projects p where p.enabled=${_true}
+    and p.id in
+    <foreach collection="ids" open="(" close=")" item="key" separator=",">
+      #{key}
     </foreach>
 
   </select>
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/ActionPlanDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/ActionPlanDaoTest.java
index aba46152c54..df72ee82542 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/db/ActionPlanDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/db/ActionPlanDaoTest.java
@@ -20,11 +20,14 @@
 
 package org.sonar.core.issue.db;
 
+import org.apache.ibatis.session.SqlSession;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.core.persistence.AbstractDaoTestCase;
+import org.sonar.core.persistence.MyBatis;
 
 import java.util.Collection;
+import java.util.List;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static org.fest.assertions.Assertions.assertThat;
@@ -86,6 +89,22 @@ public class ActionPlanDaoTest extends AbstractDaoTestCase {
     assertThat(result).hasSize(3);
   }
 
+  @Test
+  public void should_find_by_keys_on_huge_number_of_keys() {
+    setupData("shared");
+
+    SqlSession session = getMyBatis().openSession();
+    List<String> hugeNbOKeys = newArrayList();
+    for (int i=0; i<4500; i++) {
+      hugeNbOKeys.add("ABCD" + i);
+    }
+    List<ActionPlanDto> result = dao.findByKeys(hugeNbOKeys);
+    MyBatis.closeQuietly(session);
+
+    // The goal of this test is only to check that the query do no fail, not to check the number of results
+    assertThat(result).isEmpty();
+  }
+
   @Test
   public void should_find_open_by_project_id() {
     setupData("shared", "should_find_open_by_project_id");
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java
index 95e524dfe40..180684a170d 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java
@@ -71,7 +71,7 @@ public class IssueChangeDaoTest extends AbstractDaoTestCase {
 
     SqlSession session = getMyBatis().openSession();
     List<String> hugeNbOfIssues = newArrayList();
-    for (int i=0; i<1500; i++) {
+    for (int i=0; i<4500; i++) {
       hugeNbOfIssues.add("ABCD"+i);
     }
     List<DefaultIssueComment> comments = dao.selectCommentsByIssues(session, hugeNbOfIssues);
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java
index 180725f7150..69ac592d896 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java
@@ -322,7 +322,7 @@ public class IssueDaoTest extends AbstractDaoTestCase {
     setupData("shared");
 
     List<Long> hugeNbOfIssues = newArrayList();
-    for (long i=0; i<1500; i++) {
+    for (long i=0; i<4500; i++) {
       hugeNbOfIssues.add(i);
     }
     List<IssueDto> results = dao.selectByIds(hugeNbOfIssues);
diff --git a/sonar-core/src/test/java/org/sonar/core/resource/ResourceDaoTest.java b/sonar-core/src/test/java/org/sonar/core/resource/ResourceDaoTest.java
index 94e9978f2f4..1855d41a178 100644
--- a/sonar-core/src/test/java/org/sonar/core/resource/ResourceDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/resource/ResourceDaoTest.java
@@ -161,6 +161,20 @@ public class ResourceDaoTest extends AbstractDaoTestCase {
     assertThat(component.qualifier()).isNotNull();
   }
 
+  @Test
+  public void should_find_components_by_resource_ids_on_huge_number_of_ids() {
+    setupData("fixture");
+
+    List<Long> hugeNbOfIds = newArrayList();
+    for (long i=0; i<4500; i++) {
+      hugeNbOfIds.add(i);
+    }
+    Collection<Component> results = dao.findByIds(hugeNbOfIds);
+
+    // The goal of this test is only to check that the query do no fail, not to check the number of results
+    assertThat(results).isNotNull();
+  }
+
   @Test
   public void should_find_root_project_by_component_key() {
     setupData("fixture");
-- 
2.39.5