From eb2e998e0d8efafb211e864d2fc610fdef7df80f Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 14 Mar 2017 15:02:46 +0100 Subject: [PATCH] fix pagination in OrganizationDao#selectByQuery --- .../db/organization/OrganizationDao.java | 4 +- .../db/organization/OrganizationMapper.java | 2 +- .../db/organization/OrganizationMapper.xml | 7 +- .../db/organization/OrganizationDaoTest.java | 74 +++++++++---------- .../server/organization/ws/SearchAction.java | 4 +- 5 files changed, 46 insertions(+), 45 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationDao.java index 51d9d283b46..832ea21bc8b 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationDao.java @@ -47,9 +47,9 @@ public class OrganizationDao implements Dao { getMapper(dbSession).insert(organization); } - public List selectByQuery(DbSession dbSession, OrganizationQuery organizationQuery, int page, int pageSize) { + public List selectByQuery(DbSession dbSession, OrganizationQuery organizationQuery, Pagination pagination) { requireNonNull(organizationQuery, "organizationQuery can't be null"); - return getMapper(dbSession).selectByQuery(organizationQuery, page, pageSize); + return getMapper(dbSession).selectByQuery(organizationQuery, pagination); } public Optional selectByUuid(DbSession dbSession, String uuid) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationMapper.java index cffd15faa82..8e2755dfabc 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationMapper.java @@ -28,7 +28,7 @@ public interface OrganizationMapper { void insert(@Param("organization") OrganizationDto organization); List selectByQuery(@Param("query") OrganizationQuery organizationQuery, - @Param("offset") int offset, @Param("pageSize") int pageSize); + @Param("pagination") Pagination pagination); @CheckForNull OrganizationDto selectByKey(@Param("key") String key); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/organization/OrganizationMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/organization/OrganizationMapper.xml index 04913948f18..ee67444d23a 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/organization/OrganizationMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/organization/OrganizationMapper.xml @@ -62,7 +62,7 @@ order by org.created_at desc - limit #{pageSize} offset #{offset} + limit #{pagination.pageSize,jdbcType=INTEGER} offset #{pagination.offset,jdbcType=INTEGER} @@ -86,7 +86,8 @@ ORDER BY org.created_at desc ) t ) t - where t.rn between (#{offset} + 1) and ((#{offset} + 1) * #{pageSize}) + where + t.rn between #{pagination.startRowNumber,jdbcType=INTEGER} and #{pagination.endRowNumber,jdbcType=INTEGER} diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java index 3f3f431dc30..3ccd8c3b976 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java @@ -53,10 +53,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.sonar.db.organization.OrganizationQuery.newOrganizationQueryBuilder; -import static org.sonar.db.organization.OrganizationQuery.returnAll; import static org.sonar.db.Pagination.all; import static org.sonar.db.Pagination.forPage; +import static org.sonar.db.organization.OrganizationQuery.newOrganizationQueryBuilder; +import static org.sonar.db.organization.OrganizationQuery.returnAll; public class OrganizationDaoTest { private static final long SOME_DATE = 1_200_999L; @@ -304,19 +304,19 @@ public class OrganizationDaoTest { @Test public void selectByQuery_returns_empty_when_table_is_empty() { - assertThat(underTest.selectByQuery(dbSession, returnAll(), 1, 1)).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(2).andSize(1))).isEmpty(); } @Test public void selectByQuery_returns_single_row_of_table_when_requesting_first_page_of_size_1_or_more() { insertOrganization(ORGANIZATION_DTO_1); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 0, 1)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(1).andSize(1))) .hasSize(1) .extracting("uuid") .containsOnly(ORGANIZATION_DTO_1.getUuid()); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 0, 10)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(1).andSize(10))) .hasSize(1) .extracting("uuid") .containsOnly(ORGANIZATION_DTO_1.getUuid()); @@ -326,9 +326,9 @@ public class OrganizationDaoTest { public void selectByQuery_returns_empty_on_table_with_single_row_when_not_requesting_the_first_page() { insertOrganization(ORGANIZATION_DTO_1); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 1, 1)).isEmpty(); - assertThat(underTest.selectByQuery(dbSession, returnAll(), Math.abs(new Random().nextInt(10)) + 1, 1)).isEmpty(); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 1, 10)).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(2).andSize(1))).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(Math.abs(new Random().nextInt(10)) + 2).andSize(1))).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(2).andSize(10))).isEmpty(); } @Test @@ -341,40 +341,40 @@ public class OrganizationDaoTest { insertOrganization(copyOf(ORGANIZATION_DTO_1).setUuid("uuid5").setKey("key-5")); insertOrganization(copyOf(ORGANIZATION_DTO_1).setUuid("uuid4").setKey("key-4")); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 0, 1)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(1).andSize(1))) .extracting("uuid", "key") .containsExactly(tuple("uuid4", "key-4")); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 1, 1)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(2).andSize(1))) .extracting("uuid", "key") .containsExactly(tuple("uuid5", "key-5")); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 2, 1)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(3).andSize(1))) .extracting("uuid", "key") .containsExactly(tuple("uuid2", "key-2")); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 3, 1)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(4).andSize(1))) .extracting("uuid", "key") .containsExactly(tuple("uuid1", "key-1")); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 4, 1)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(5).andSize(1))) .extracting("uuid", "key") .containsExactly(tuple("uuid3", "key-3")); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 5, 1)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(6).andSize(1))) .isEmpty(); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 0, 5)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(1).andSize(5))) .extracting("uuid") .containsExactly("uuid4", "uuid5", "uuid2", "uuid1", "uuid3"); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 5, 5)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(6).andSize(5))) .isEmpty(); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 0, 3)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(1).andSize(3))) .extracting("uuid") .containsExactly("uuid4", "uuid5", "uuid2"); - assertThat(underTest.selectByQuery(dbSession, returnAll(), 3, 3)) + assertThat(underTest.selectByQuery(dbSession, returnAll(), forPage(2).andSize(3))) .extracting("uuid") .containsExactly("uuid1", "uuid3"); } @Test public void selectByQuery_with_keys_returns_empty_when_table_is_empty() { - assertThat(underTest.selectByQuery(dbSession, newQueryWithKeys("key1", "key2"), 1, 1)) + assertThat(underTest.selectByQuery(dbSession, newQueryWithKeys("key1", "key2"), forPage(2).andSize(1))) .isEmpty(); } @@ -384,10 +384,10 @@ public class OrganizationDaoTest { insertOrganization(ORGANIZATION_DTO_2); OrganizationQuery organizationQuery = newQueryWithKeys(ORGANIZATION_DTO_1.getKey(), ORGANIZATION_DTO_2.getKey()); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, 0, 1)) + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(1).andSize(1))) .hasSize(1); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, 0, 10)) + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(1).andSize(10))) .hasSize(2) .extracting(OrganizationDto::getUuid) .containsOnly(ORGANIZATION_DTO_1.getUuid(), ORGANIZATION_DTO_2.getUuid()); @@ -399,7 +399,7 @@ public class OrganizationDaoTest { insertOrganization(ORGANIZATION_DTO_2); OrganizationQuery organizationQuery = newOrganizationQueryBuilder().setKeys(Lists.emptyList()).build(); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, 0, 10)) + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(1).andSize(10))) .extracting(OrganizationDto::getUuid) .containsOnly(ORGANIZATION_DTO_1.getUuid(), ORGANIZATION_DTO_2.getUuid()); } @@ -410,7 +410,7 @@ public class OrganizationDaoTest { insertOrganization(ORGANIZATION_DTO_2); OrganizationQuery organizationQuery = newQueryWithKeys(PERMISSION_1, PERMISSION_2, "dog"); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, 0, 10)) + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(1).andSize(10))) .isEmpty(); } @@ -420,7 +420,7 @@ public class OrganizationDaoTest { insertOrganization(ORGANIZATION_DTO_2); OrganizationQuery organizationQuery = newQueryWithKeys(ORGANIZATION_DTO_1.getKey(), PERMISSION_1, ORGANIZATION_DTO_2.getKey(), PERMISSION_2, "dog"); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, 0, 10)) + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(1).andSize(10))) .hasSize(2) .extracting(OrganizationDto::getUuid) .containsOnly(ORGANIZATION_DTO_1.getUuid(), ORGANIZATION_DTO_2.getUuid()); @@ -432,9 +432,9 @@ public class OrganizationDaoTest { insertOrganization(ORGANIZATION_DTO_2); OrganizationQuery organizationQuery = newQueryWithKeys(ORGANIZATION_DTO_1.getKey(), ORGANIZATION_DTO_2.getKey()); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, 2, 2)).isEmpty(); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, Math.abs(new Random().nextInt(10)) + 2, 1)).isEmpty(); - assertThat(underTest.selectByQuery(dbSession, organizationQuery, 2, 10)).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(2).andSize(2))).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(Math.abs(new Random().nextInt(10)) + 3).andSize(1))).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, organizationQuery, forPage(3).andSize(10))).isEmpty(); } @Test @@ -448,33 +448,33 @@ public class OrganizationDaoTest { insertOrganization(copyOf(ORGANIZATION_DTO_1).setUuid("uuid4").setKey("key-4")); OrganizationQuery allExistingKeys = newQueryWithKeys("key-1", "key-2", "key-3", "key-4", "key-5"); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 0, 1)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(1).andSize(1))) .extracting(OrganizationDto::getUuid, OrganizationDto::getKey) .containsExactly(tuple("uuid4", "key-4")); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 1, 1)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(2).andSize(1))) .extracting(OrganizationDto::getUuid, OrganizationDto::getKey) .containsExactly(tuple("uuid5", "key-5")); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 2, 1)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(3).andSize(1))) .extracting(OrganizationDto::getUuid, OrganizationDto::getKey) .containsExactly(tuple("uuid2", "key-2")); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 3, 1)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(4).andSize(1))) .extracting(OrganizationDto::getUuid, OrganizationDto::getKey) .containsExactly(tuple("uuid1", "key-1")); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 4, 1)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(5).andSize(1))) .extracting(OrganizationDto::getUuid, OrganizationDto::getKey) .containsExactly(tuple("uuid3", "key-3")); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 5, 1)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(6).andSize(1))) .isEmpty(); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 0, 5)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(1).andSize(5))) .extracting(OrganizationDto::getUuid) .containsExactly("uuid4", "uuid5", "uuid2", "uuid1", "uuid3"); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 5, 5)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(2).andSize(5))) .isEmpty(); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 0, 3)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(1).andSize(3))) .extracting(OrganizationDto::getUuid) .containsExactly("uuid4", "uuid5", "uuid2"); - assertThat(underTest.selectByQuery(dbSession, allExistingKeys, 3, 3)) + assertThat(underTest.selectByQuery(dbSession, allExistingKeys, forPage(2).andSize(3))) .extracting(OrganizationDto::getUuid) .containsExactly("uuid1", "uuid3"); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/SearchAction.java index 254c10fc451..902ed5a5dfa 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/SearchAction.java @@ -32,6 +32,7 @@ import org.sonar.db.organization.OrganizationQuery; import org.sonarqube.ws.Organizations; import org.sonarqube.ws.Organizations.Organization; +import static org.sonar.db.Pagination.forPage; import static org.sonar.db.organization.OrganizationQuery.newOrganizationQueryBuilder; import static org.sonar.server.ws.WsUtils.writeProtobuf; @@ -79,8 +80,7 @@ public class SearchAction implements OrganizationsWsAction { List dtos = dbClient.organizationDao().selectByQuery( dbSession, organizationQuery, - paging.offset(), - paging.pageSize()); + forPage(paging.pageIndex()).andSize(paging.pageSize())); writeResponse(request, response, dtos); } -- 2.39.5