]> source.dussan.org Git - sonarqube.git/commitdiff
fix pagination in OrganizationDao#selectByQuery
authorSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Tue, 14 Mar 2017 14:02:46 +0000 (15:02 +0100)
committerSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Thu, 23 Mar 2017 16:38:34 +0000 (17:38 +0100)
server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/organization/OrganizationMapper.java
server/sonar-db-dao/src/main/resources/org/sonar/db/organization/OrganizationMapper.xml
server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java
server/sonar-server/src/main/java/org/sonar/server/organization/ws/SearchAction.java

index 51d9d283b460a6ff36ae83a6bd5084c5b82ed685..832ea21bc8b4e6fdf5cda8bf5a92d6cc7fd93476 100644 (file)
@@ -47,9 +47,9 @@ public class OrganizationDao implements Dao {
     getMapper(dbSession).insert(organization);
   }
 
-  public List<OrganizationDto> selectByQuery(DbSession dbSession, OrganizationQuery organizationQuery, int page, int pageSize) {
+  public List<OrganizationDto> 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<OrganizationDto> selectByUuid(DbSession dbSession, String uuid) {
index cffd15faa829f0f56d028de93e07df6c73f7fd95..8e2755dfabcfaedff2e16bc0a40aad2c0991c630 100644 (file)
@@ -28,7 +28,7 @@ public interface OrganizationMapper {
   void insert(@Param("organization") OrganizationDto organization);
 
   List<OrganizationDto> selectByQuery(@Param("query") OrganizationQuery organizationQuery,
-    @Param("offset") int offset, @Param("pageSize") int pageSize);
+    @Param("pagination") Pagination pagination);
 
   @CheckForNull
   OrganizationDto selectByKey(@Param("key") String key);
index 04913948f182f4ed6d247a4d2c604eaecab40d5f..ee67444d23ae33dae990187e06e60f814b688741 100644 (file)
@@ -62,7 +62,7 @@
     <include refid="sqlSelectByQuery" />
     order by
       org.created_at desc
-    limit #{pageSize} offset #{offset}
+    limit #{pagination.pageSize,jdbcType=INTEGER} offset #{pagination.offset,jdbcType=INTEGER}
   </select>
 
   <select id="selectByQuery" parameterType="map" resultType="Organization" databaseId="mssql">
@@ -72,7 +72,7 @@
       <include refid="sqlSelectByQuery" />
     ) as query
     where
-      query.number between (#{offset} + 1) and ((#{offset} + 1) * #{pageSize})
+      query.number between #{pagination.startRowNumber,jdbcType=INTEGER} and #{pagination.endRowNumber,jdbcType=INTEGER}
     order by
       query.createdAt desc
   </select>
@@ -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}
   </select>
 
   <sql id="sqlSelectByQuery">
index 3f3f431dc304b5d7eae099ba868ce67c3c0402c8..3ccd8c3b976190dd47f459028302f8e616ac5fe5 100644 (file)
@@ -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");
   }
index 254c10fc451bc20b4b87002adfc1704a3f19ef9f..902ed5a5dfa2f3ac40424d6df74030075b44ac83 100644 (file)
@@ -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<OrganizationDto> dtos = dbClient.organizationDao().selectByQuery(
         dbSession,
         organizationQuery,
-        paging.offset(),
-        paging.pageSize());
+        forPage(paging.pageIndex()).andSize(paging.pageSize()));
 
       writeResponse(request, response, dtos);
     }