From 175976346d83d4eb1f566123e6e31d4e4bfae969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 14 Mar 2017 15:42:05 +0100 Subject: [PATCH] fix and simplify pagination CeActivityDao#selectByQuery --- .../java/org/sonar/db/ce/CeActivityDao.java | 5 +- .../org/sonar/db/ce/CeActivityMapper.java | 3 +- .../org/sonar/db/ce/CeActivityMapper.xml | 30 ++++++------ .../org/sonar/db/ce/CeActivityDaoTest.java | 49 ++++++++++--------- .../sonar/server/ce/ws/ActivityAction.java | 3 +- .../sonar/server/ce/ws/ComponentAction.java | 3 +- .../server/ce/ws/ActivityActionTest.java | 13 ++++- 7 files changed, 61 insertions(+), 45 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityDao.java index a0dc3ee387ec..0e8f678fd65a 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityDao.java +++ b/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 uuids) { /** * Ordered by id desc -> newest to oldest */ - public List selectByQuery(DbSession dbSession, CeTaskQuery query, int offset, int pageSize) { + public List 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) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityMapper.java index e8838e782f2b..a741410266b4 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/ce/CeActivityMapper.java +++ b/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 selectByComponentUuid(@Param("componentUuid") String componentUuid); - List selectByQuery(@Param("query") CeTaskQuery query, @Param("offset") int offset, @Param("pageSize") int pageSize); + List selectByQuery(@Param("query") CeTaskQuery query, @Param("pagination") Pagination pagination); List selectOlderThan(@Param("beforeDate") long beforeDate); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/ce/CeActivityMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/ce/CeActivityMapper.xml index bac934d732f1..6324e2fca436 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/ce/CeActivityMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/ce/CeActivityMapper.xml @@ -45,33 +45,35 @@ diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/ce/CeActivityDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/ce/CeActivityDaoTest.java index 142344fbf126..44853151c4ca 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/ce/CeActivityDaoTest.java +++ b/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.emptyList()); - List dtos = underTest.selectByQuery(db.getSession(), query, 0, 10); + List 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 dtos = underTest.selectByQuery(db.getSession(), new CeTaskQuery().setComponentUuid("PROJECT_1"), 0, 100); + List 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,14 +213,13 @@ 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 @@ -226,8 +227,8 @@ 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.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 selectPageOfUuids(int offset, int pageSize) { - return underTest.selectByQuery(db.getSession(), new CeTaskQuery(), offset, pageSize).stream() + private List selectPageOfUuids(Pagination pagination) { + return underTest.selectByQuery(db.getSession(), new CeTaskQuery(), pagination).stream() .map(CeActivityToUuid.INSTANCE::apply) .collect(Collectors.toList()); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java index cbfeae84bc7e..d38e7a1570d5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java +++ b/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 loadQueuedTasks(DbSession dbSession, ActivityWsRequest r } private List loadPastTasks(DbSession dbSession, ActivityWsRequest request, CeTaskQuery query) { - List dtos = dbClient.ceActivityDao().selectByQuery(dbSession, query, OFFSET, request.getPageSize()); + List dtos = dbClient.ceActivityDao().selectByQuery(dbSession, query, forPage(1).andSize(request.getPageSize())); return formatter.formatActivity(dbSession, dtos); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java index d97b2c662b4f..013516ad9839 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java +++ b/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 activityDtos = dbClient.ceActivityDao().selectByQuery(dbSession, activityQuery, 0, 1); + List activityDtos = dbClient.ceActivityDao().selectByQuery(dbSession, activityQuery, forPage(1).andSize(1)); ProjectResponse.Builder wsResponseBuilder = ProjectResponse.newBuilder(); wsResponseBuilder.addAllQueue(formatter.formatQueue(dbSession, queueDtos)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java index 16b592cbd332..20129c4635e5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java +++ b/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 expectedOrderedTaskIds) {