fix and simplify pagination CeActivityDao#selectByQuery

server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityDao.java

@@ -27,6 +27,7 @@
 import org.sonar.api.utils.System2;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
+import org.sonar.db.Pagination;
 
 import static org.sonar.db.DatabaseUtils.executeLargeUpdates;
 
@@ -65,12 +66,12 @@ public void deleteByUuids(DbSession dbSession, Set<String> uuids) {
   /**
    * Ordered by id desc -> newest to oldest
    */
-  public List<CeActivityDto> selectByQuery(DbSession dbSession, CeTaskQuery query, int offset, int pageSize) {
+  public List<CeActivityDto> selectByQuery(DbSession dbSession, CeTaskQuery query, Pagination pagination) {
     if (query.isShortCircuitedByComponentUuids()) {
       return Collections.emptyList();
     }
 
-    return mapper(dbSession).selectByQuery(query, offset, pageSize);
+    return mapper(dbSession).selectByQuery(query, pagination);
   }
 
   public int countLastByStatusAndComponentUuid(DbSession dbSession, CeActivityDto.Status status, @Nullable String componentUuid) {

server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityMapper.java

@@ -23,6 +23,7 @@
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.ibatis.annotations.Param;
+import org.sonar.db.Pagination;
 
 public interface CeActivityMapper {
 
@@ -31,7 +32,7 @@ public interface CeActivityMapper {
 
   List<CeActivityDto> selectByComponentUuid(@Param("componentUuid") String componentUuid);
 
-  List<CeActivityDto> selectByQuery(@Param("query") CeTaskQuery query, @Param("offset") int offset, @Param("pageSize") int pageSize);
+  List<CeActivityDto> selectByQuery(@Param("query") CeTaskQuery query, @Param("pagination") Pagination pagination);
 
   List<CeActivityDto> selectOlderThan(@Param("beforeDate") long beforeDate);
 

server/sonar-db-dao/src/main/resources/org/sonar/db/ce/CeActivityMapper.xml

@@ -45,33 +45,35 @@
   </select>
 
   <select id="selectByQuery" parameterType="map" resultType="org.sonar.db.ce.CeActivityDto">
-    SELECT
-    <include refid="columns"/>
+    select
+      <include refid="columns"/>
     <include refid="sqlSelectByQuery" />
-    ORDER BY ca.id desc
-    LIMIT #{pageSize} OFFSET #{offset}
+    order by ca.id desc
+    limit #{pagination.pageSize,jdbcType=INTEGER} offset #{pagination.offset,jdbcType=INTEGER}
   </select>
 
   <select id="selectByQuery" parameterType="map" resultType="org.sonar.db.ce.CeActivityDto" databaseId="mssql">
-    SELECT * FROM (
-    SELECT ROW_NUMBER() OVER(ORDER BY id desc) AS NUMBER,
+    select * from (
+    select row_number() over(order by id desc) as number,
       <include refid="columns"/>
       <include refid="sqlSelectByQuery" />
-      ) AS QUERY
-    WHERE NUMBER BETWEEN (#{offset} + 1) AND ((#{offset} + 1) * #{pageSize})
-    ORDER BY id desc
+      ) as query
+    where
+      query.number between #{pagination.startRowNumber,jdbcType=INTEGER} and #{pagination.endRowNumber,jdbcType=INTEGER}
+    order by id desc
   </select>
 
   <select id="selectByQuery" parameterType="map" resultType="org.sonar.db.ce.CeActivityDto" databaseId="oracle">
-    SELECT * FROM (
-      SELECT ROWNUM AS rn, t.* FROM (
-        SELECT
+    select * from (
+      select rownum as rn, t.* from (
+        select
         <include refid="columns"/>
         <include refid="sqlSelectByQuery" />
-        ORDER BY ca.id desc
+        order by ca.id desc
       ) t
     ) t
-    WHERE rn BETWEEN (#{offset} + 1) AND ((#{offset} + 1) * #{pageSize})
+    where
+      t.rn between #{pagination.startRowNumber,jdbcType=INTEGER} and #{pagination.endRowNumber,jdbcType=INTEGER}
   </select>
 
   <sql id="sqlSelectByQuery">

server/sonar-db-dao/src/test/java/org/sonar/db/ce/CeActivityDaoTest.java

@@ -33,10 +33,12 @@
 import org.sonar.core.util.stream.Collectors;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
+import org.sonar.db.Pagination;
 
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.db.Pagination.forPage;
 import static org.sonar.db.ce.CeActivityDto.Status.FAILED;
 import static org.sonar.db.ce.CeActivityDto.Status.SUCCESS;
 import static org.sonar.db.ce.CeTaskTypes.REPORT;
@@ -153,30 +155,30 @@ public void test_selectByQuery() {
 
     // no filters
     CeTaskQuery query = new CeTaskQuery().setStatuses(Collections.<String>emptyList());
-    List<CeActivityDto> dtos = underTest.selectByQuery(db.getSession(), query, 0, 10);
+    List<CeActivityDto> dtos = underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(10));
     assertThat(dtos).extracting("uuid").containsExactly("TASK_4", "TASK_3", "TASK_2", "TASK_1");
 
     // select by component uuid
     query = new CeTaskQuery().setComponentUuid("PROJECT_1");
-    dtos = underTest.selectByQuery(db.getSession(), query, 0, 100);
+    dtos = underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(100));
     assertThat(dtos).extracting("uuid").containsExactly("TASK_2", "TASK_1");
 
     // select by status
     query = new CeTaskQuery().setStatuses(singletonList(CeActivityDto.Status.SUCCESS.name()));
-    dtos = underTest.selectByQuery(db.getSession(), query, 0, 100);
+    dtos = underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(100));
     assertThat(dtos).extracting("uuid").containsExactly("TASK_4", "TASK_3", "TASK_1");
 
     // select by type
     query = new CeTaskQuery().setType(REPORT);
-    dtos = underTest.selectByQuery(db.getSession(), query, 0, 100);
+    dtos = underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(100));
     assertThat(dtos).extracting("uuid").containsExactly("TASK_3", "TASK_2", "TASK_1");
     query = new CeTaskQuery().setType("views");
-    dtos = underTest.selectByQuery(db.getSession(), query, 0, 100);
+    dtos = underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(100));
     assertThat(dtos).extracting("uuid").containsExactly("TASK_4");
 
     // select by multiple conditions
     query = new CeTaskQuery().setType(REPORT).setOnlyCurrents(true).setComponentUuid("PROJECT_1");
-    dtos = underTest.selectByQuery(db.getSession(), query, 0, 100);
+    dtos = underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(100));
     assertThat(dtos).extracting("uuid").containsExactly("TASK_2");
   }
 
@@ -185,7 +187,7 @@ public void selectByQuery_does_not_populate_errorStacktrace_field() {
     insert("TASK_1", REPORT, "PROJECT_1", FAILED);
     underTest.insert(db.getSession(), createActivityDto("TASK_2", REPORT, "PROJECT_1", FAILED).setErrorStacktrace("some stack"));
 
-    List<CeActivityDto> dtos = underTest.selectByQuery(db.getSession(), new CeTaskQuery().setComponentUuid("PROJECT_1"), 0, 100);
+    List<CeActivityDto> dtos = underTest.selectByQuery(db.getSession(), new CeTaskQuery().setComponentUuid("PROJECT_1"), forPage(1).andSize(100));
 
     assertThat(dtos)
       .hasSize(2)
@@ -198,9 +200,9 @@ public void selectByQuery_populates_hasScannerContext_flag() {
     CeActivityDto dto2 = insert("TASK_2", REPORT, "PROJECT_2", SUCCESS);
     insertScannerContext(dto2.getUuid());
 
-    CeActivityDto dto = underTest.selectByQuery(db.getSession(), new CeTaskQuery().setComponentUuid("PROJECT_1"), 0, 100).iterator().next();
+    CeActivityDto dto = underTest.selectByQuery(db.getSession(), new CeTaskQuery().setComponentUuid("PROJECT_1"), forPage(1).andSize(100)).iterator().next();
     assertThat(dto.isHasScannerContext()).isFalse();
-    dto = underTest.selectByQuery(db.getSession(), new CeTaskQuery().setComponentUuid("PROJECT_2"), 0, 100).iterator().next();
+    dto = underTest.selectByQuery(db.getSession(), new CeTaskQuery().setComponentUuid("PROJECT_2"), forPage(1).andSize(100)).iterator().next();
     assertThat(dto.isHasScannerContext()).isTrue();
   }
 
@@ -211,23 +213,22 @@ public void selectByQuery_is_paginated_and_return_results_sorted_from_last_to_fi
     insert("TASK_3", REPORT, "PROJECT_2", CeActivityDto.Status.SUCCESS);
     insert("TASK_4", "views", null, CeActivityDto.Status.SUCCESS);
 
-    assertThat(selectPageOfUuids(0, 1)).containsExactly("TASK_4");
-    assertThat(selectPageOfUuids(1, 1)).containsExactly("TASK_3");
-    assertThat(selectPageOfUuids(0, 3)).containsExactly("TASK_4", "TASK_3", "TASK_2");
-    assertThat(selectPageOfUuids(0, 4)).containsExactly("TASK_4", "TASK_3", "TASK_2", "TASK_1");
-    assertThat(selectPageOfUuids(3, 4)).containsExactly("TASK_1");
-    assertThat(selectPageOfUuids(0, 100)).containsExactly("TASK_4", "TASK_3", "TASK_2", "TASK_1");
-    assertThat(selectPageOfUuids(0, 0)).isEmpty();
-    assertThat(selectPageOfUuids(10, 2)).isEmpty();
+    assertThat(selectPageOfUuids(forPage(1).andSize(1))).containsExactly("TASK_4");
+    assertThat(selectPageOfUuids(forPage(2).andSize(1))).containsExactly("TASK_3");
+    assertThat(selectPageOfUuids(forPage(1).andSize(3))).containsExactly("TASK_4", "TASK_3", "TASK_2");
+    assertThat(selectPageOfUuids(forPage(1).andSize(4))).containsExactly("TASK_4", "TASK_3", "TASK_2", "TASK_1");
+    assertThat(selectPageOfUuids(forPage(2).andSize(3))).containsExactly("TASK_1");
+    assertThat(selectPageOfUuids(forPage(1).andSize(100))).containsExactly("TASK_4", "TASK_3", "TASK_2", "TASK_1");
+    assertThat(selectPageOfUuids(forPage(5).andSize(2))).isEmpty();
   }
 
   @Test
   public void selectByQuery_no_results_if_shortcircuited_by_component_uuids() {
     insert("TASK_1", REPORT, "PROJECT_1", CeActivityDto.Status.SUCCESS);
 
     CeTaskQuery query = new CeTaskQuery();
-    query.setComponentUuids(Collections.<String>emptyList());
-    assertThat(underTest.selectByQuery(db.getSession(), query, 0, 0)).isEmpty();
+    query.setComponentUuids(Collections.emptyList());
+    assertThat(underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(1))).isEmpty();
   }
 
   @Test
@@ -237,17 +238,17 @@ public void select_and_count_by_date() {
 
     // search by min submitted date
     CeTaskQuery query = new CeTaskQuery().setMinSubmittedAt(1_455_000_000_000L);
-    assertThat(underTest.selectByQuery(db.getSession(), query, 0, 5)).extracting("uuid").containsOnly("UUID2");
+    assertThat(underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(5))).extracting("uuid").containsOnly("UUID2");
 
     // search by max executed date
     query = new CeTaskQuery().setMaxExecutedAt(1_475_000_000_000L);
-    assertThat(underTest.selectByQuery(db.getSession(), query, 0, 5)).extracting("uuid").containsOnly("UUID1");
+    assertThat(underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(5))).extracting("uuid").containsOnly("UUID1");
 
     // search by both dates
     query = new CeTaskQuery()
       .setMinSubmittedAt(1_400_000_000_000L)
       .setMaxExecutedAt(1_475_000_000_000L);
-    assertThat(underTest.selectByQuery(db.getSession(), query, 0, 5)).extracting("uuid").containsOnly("UUID1");
+    assertThat(underTest.selectByQuery(db.getSession(), query, forPage(1).andSize(5))).extracting("uuid").containsOnly("UUID1");
 
   }
 
@@ -377,8 +378,8 @@ private void insertScannerContext(String taskUuid) {
     dbSession.commit();
   }
 
-  private List<String> selectPageOfUuids(int offset, int pageSize) {
-    return underTest.selectByQuery(db.getSession(), new CeTaskQuery(), offset, pageSize).stream()
+  private List<String> selectPageOfUuids(Pagination pagination) {
+    return underTest.selectByQuery(db.getSession(), new CeTaskQuery(), pagination).stream()
         .map(CeActivityToUuid.INSTANCE::apply)
         .collect(Collectors.toList());
   }

server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java

@@ -56,6 +56,7 @@
 import static org.apache.commons.lang.StringUtils.defaultString;
 import static org.sonar.api.utils.DateUtils.parseEndingDateOrDateTime;
 import static org.sonar.api.utils.DateUtils.parseStartingDateOrDateTime;
+import static org.sonar.db.Pagination.forPage;
 import static org.sonar.server.ws.WsUtils.checkRequest;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
 import static org.sonarqube.ws.client.ce.CeWsParameters.PARAM_COMPONENT_ID;
@@ -254,7 +255,7 @@ private List<WsCe.Task> loadQueuedTasks(DbSession dbSession, ActivityWsRequest r
   }
 
   private List<WsCe.Task> loadPastTasks(DbSession dbSession, ActivityWsRequest request, CeTaskQuery query) {
-    List<CeActivityDto> dtos = dbClient.ceActivityDao().selectByQuery(dbSession, query, OFFSET, request.getPageSize());
+    List<CeActivityDto> dtos = dbClient.ceActivityDao().selectByQuery(dbSession, query, forPage(1).andSize(request.getPageSize()));
     return formatter.formatActivity(dbSession, dtos);
   }
 

server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java

@@ -35,6 +35,7 @@
 import org.sonar.server.user.UserSession;
 import org.sonar.server.ws.KeyExamples;
 
+import static org.sonar.db.Pagination.forPage;
 import static org.sonar.server.component.ComponentFinder.ParamNames.COMPONENT_ID_AND_KEY;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
 import static org.sonarqube.ws.WsCe.ProjectResponse;
@@ -86,7 +87,7 @@ public void handle(Request wsRequest, Response wsResponse) throws Exception {
       CeTaskQuery activityQuery = new CeTaskQuery()
         .setComponentUuid(component.uuid())
         .setOnlyCurrents(true);
-      List<CeActivityDto> activityDtos = dbClient.ceActivityDao().selectByQuery(dbSession, activityQuery, 0, 1);
+      List<CeActivityDto> activityDtos = dbClient.ceActivityDao().selectByQuery(dbSession, activityQuery, forPage(1).andSize(1));
 
       ProjectResponse.Builder wsResponseBuilder = ProjectResponse.newBuilder();
       wsResponseBuilder.addAllQueue(formatter.formatQueue(dbSession, queueDtos));

server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java

@@ -21,7 +21,6 @@
 
 import com.google.common.base.Throwables;
 import java.io.IOException;
-import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import org.junit.Rule;
@@ -186,7 +185,17 @@ public void limit_results() {
     assertPage(1, asList("T3"));
     assertPage(2, asList("T3", "T2"));
     assertPage(10, asList("T3", "T2", "T1"));
-    assertPage(0, Collections.emptyList());
+  }
+
+  @Test
+  public void throws_IAE_if_pageSize_is_0() {
+    logInAsSystemAdministrator();
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("page size must be >= 1");
+
+    call(ws.newRequest()
+      .setParam(Param.PAGE_SIZE, Integer.toString(0))
+      .setParam(PARAM_STATUS, "SUCCESS,FAILED,CANCELED,IN_PROGRESS,PENDING"));
   }
 
   private void assertPage(int pageSize, List<String> expectedOrderedTaskIds) {