diff options
12 files changed, 206 insertions, 59 deletions
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueDao.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueDao.java index 941bf0873ae..3f2add7205f 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueDao.java @@ -22,7 +22,6 @@ package org.sonar.core.issue; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; -import org.apache.ibatis.session.RowBounds; import org.apache.ibatis.session.SqlSession; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; @@ -30,6 +29,7 @@ import org.sonar.api.issue.IssueQuery; import org.sonar.core.persistence.MyBatis; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -102,28 +102,24 @@ public class IssueDao implements BatchComponent, ServerComponent { public List<IssueDto> select(IssueQuery query) { SqlSession session = mybatis.openSession(); try { - return select(query, session); + return session.selectList("org.sonar.core.issue.IssueMapper.select", query); } finally { MyBatis.closeQuietly(session); } } - public List<IssueDto> select(IssueQuery query, SqlSession session) { - // TODO support ordering - - return session.selectList("org.sonar.core.issue.IssueMapper.select", query, new RowBounds(query.offset(), query.limit())); - } - /** * The returned IssueDto list contains only the issue id and the resource id */ public List<IssueDto> selectIssueIdsAndComponentsId(IssueQuery query, SqlSession session) { // TODO support ordering - return session.selectList("org.sonar.core.issue.IssueMapper.selectIssueIdsAndComponentsId", query); } public Collection<IssueDto> selectByIds(Collection<Long> ids, SqlSession session) { + if (ids.isEmpty()) { + return Collections.emptyList(); + } List <List<Long>> idsPartition = Lists.partition(newArrayList(ids), 1000); Map<String, List <List<Long>>> params = ImmutableMap.of("ids", idsPartition); return session.selectList("org.sonar.core.issue.IssueMapper.selectByIds", params); 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 5e5d0f8d435..79e5d1b13c0 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 @@ -19,12 +19,14 @@ */ package org.sonar.core.resource; +import com.google.common.collect.Lists; import org.apache.ibatis.session.SqlSession; import org.sonar.api.component.Component; import org.sonar.core.component.ComponentDto; import org.sonar.core.persistence.MyBatis; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.List; @@ -133,9 +135,13 @@ public class ResourceDao { } public Collection<Component> findByIds(Collection<Integer> ids) { + if (ids.isEmpty()) { + return Collections.emptyList(); + } SqlSession session = mybatis.openSession(); try { - Collection<ResourceDto> resources = session.getMapper(ResourceMapper.class).selectResourcesById(ids); + List <List<Integer>> idsPartition = Lists.partition(newArrayList(ids), 1000); + Collection<ResourceDto> resources = session.getMapper(ResourceMapper.class).selectResourcesById(idsPartition); 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 a5a1e603f7b..13093cce06c 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 @@ -22,7 +22,6 @@ package org.sonar.core.resource; import org.apache.ibatis.annotations.Param; import org.apache.ibatis.session.ResultHandler; -import java.util.Collection; import java.util.List; public interface ResourceMapper { @@ -54,7 +53,7 @@ public interface ResourceMapper { /** * @since3.6 */ - List<ResourceDto> selectResourcesById(@Param("ids") Collection<Integer> ids); + List<ResourceDto> selectResourcesById(@Param("ids") List <List<Integer>> ids); void insert(ResourceDto resource); diff --git a/sonar-core/src/main/java/org/sonar/core/rule/DefaultRuleFinder.java b/sonar-core/src/main/java/org/sonar/core/rule/DefaultRuleFinder.java index 50996b16219..667549554be 100644 --- a/sonar-core/src/main/java/org/sonar/core/rule/DefaultRuleFinder.java +++ b/sonar-core/src/main/java/org/sonar/core/rule/DefaultRuleFinder.java @@ -31,6 +31,7 @@ import org.sonar.jpa.session.DatabaseSessionFactory; import javax.persistence.Query; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -57,6 +58,9 @@ public class DefaultRuleFinder implements RuleFinder { } public Collection<Rule> findByIds(Collection<Integer> ruleIds) { + if (ruleIds.isEmpty()) { + return Collections.emptyList(); + } DatabaseSession session = sessionFactory.getSession(); StringBuilder hql = new StringBuilder().append("from ").append(Rule.class.getSimpleName()).append(" r where r.id in (:ids) and status<>:status "); Query hqlQuery = session.createQuery(hql.toString()) 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 24482461298..ded87d52b8f 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,9 +76,13 @@ </select> <select id="selectResourcesById" parameterType="map" resultMap="resourceResultMap"> - select * from projects p where p.enabled=${_true} and p.id in - <foreach item="id" index="index" collection="ids" open="(" separator="," close=")">#{id} + 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> </foreach> + </select> <select id="selectSnapshot" parameterType="long" resultMap="snapshotResultMap"> diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java index e3c3b5a62ce..d40120f32db 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java @@ -47,6 +47,8 @@ public interface IssueFinder extends ServerComponent { Component component(Issue issue); Collection<Component> components(); + + Pagination pagination(); } Results find(IssueQuery query, @Nullable Integer currentUserId, String role); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java index 4ad5bc1965b..b481db42961 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java @@ -116,16 +116,8 @@ public class IssueQuery { return limit; } -// public int pages(int count) { -// int p = (count / limit); -// if ((count % limit) > 0) { -// p++; -// } -// return p; -// } - - public int offset(){ - return (page - 1) * limit; + public int page() { + return page; } @Override diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Pagination.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Pagination.java new file mode 100644 index 00000000000..bacb6d2d637 --- /dev/null +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Pagination.java @@ -0,0 +1,62 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.api.issue; + +public class Pagination { + + private int limit; + private int page; + private int count; + + public Pagination(int limit, int page, int count) { + this.limit = limit; + this.page = page; + this.count = count; + } + + public int page() { + return page; + } + + public int limit() { + return limit; + } + + public int count() { + return count; + } + + public int offset(){ + return (page - 1) * limit; + } + + public int pages() { + int p = (count / limit); + if ((count % limit) > 0) { + p++; + } + return p; + } + + public boolean isEmpty() { + return count == 0; + } +} diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java index dd1a2a38990..8f46066a642 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java @@ -59,6 +59,6 @@ public class IssueQueryTest { assertThat(query.createdAfter()).isNotNull(); assertThat(query.createdBefore()).isNotNull(); assertThat(query.limit()).isEqualTo(10); - assertThat(query.offset()).isEqualTo(10); + assertThat(query.page()).isEqualTo(2); } } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/PaginationTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/PaginationTest.java new file mode 100644 index 00000000000..f93ab52ea5b --- /dev/null +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/PaginationTest.java @@ -0,0 +1,50 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.api.issue; + +import org.junit.Test; + +import static org.fest.assertions.Assertions.assertThat; + +public class PaginationTest { + + @Test + public void test_pagination(){ + Pagination pagination = new Pagination(5, 1, 20); + + assertThat(pagination.limit()).isEqualTo(5); + assertThat(pagination.page()).isEqualTo(1); + assertThat(pagination.count()).isEqualTo(20); + assertThat(pagination.isEmpty()).isFalse(); + + assertThat(pagination.offset()).isEqualTo(0); + assertThat(pagination.pages()).isEqualTo(4); + } + + @Test + public void test_pagination_on_second_page(){ + Pagination pagination = new Pagination(5, 2, 20); + + assertThat(pagination.offset()).isEqualTo(5); + assertThat(pagination.pages()).isEqualTo(4); + } + +} diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java index caeeb420aa7..cfe08b790d9 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java @@ -21,7 +21,6 @@ package org.sonar.server.issue; import com.google.common.base.Predicate; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import com.google.common.collect.Sets; import org.apache.ibatis.session.SqlSession; import org.slf4j.Logger; @@ -30,6 +29,7 @@ import org.sonar.api.component.Component; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; +import org.sonar.api.issue.Pagination; import org.sonar.api.rules.Rule; import org.sonar.core.issue.IssueDao; import org.sonar.core.issue.IssueDto; @@ -45,6 +45,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Maps.newHashMap; /** @@ -72,43 +73,53 @@ public class ServerIssueFinder implements IssueFinder { SqlSession sqlSession = myBatis.openSession(); try { List<IssueDto> allIssuesDto = issueDao.selectIssueIdsAndComponentsId(query, sqlSession); - - Set<Integer> componentIds = Sets.newLinkedHashSet(); - for (IssueDto issueDto : allIssuesDto) { - componentIds.add(issueDto.getResourceId()); - } + Set<Integer> componentIds = getResourceIds(allIssuesDto); Set<Integer> authorizedComponentIds = authorizationDao.keepAuthorizedComponentIds(componentIds, currentUserId, role, sqlSession); - Set<Long> issueIds = getAutorizedIssueIds(allIssuesDto, authorizedComponentIds, query); + List<IssueDto> authorizedIssues = getAuthorizedIssues(allIssuesDto, authorizedComponentIds); + Pagination pagination = new Pagination(query.limit(), query.page(), authorizedIssues.size()); + Set<Long> paginatedAuthorizedIssueIds = getPaginatedAuthorizedIssueIds(authorizedIssues, pagination); - Collection<IssueDto> dtos = issueDao.selectByIds(issueIds, sqlSession); + Collection<IssueDto> dtos = issueDao.selectByIds(paginatedAuthorizedIssueIds, sqlSession); Set<Integer> ruleIds = Sets.newLinkedHashSet(); - List<Issue> issues = Lists.newArrayList(); + List<Issue> issues = newArrayList(); for (IssueDto dto : dtos) { if (authorizedComponentIds.contains(dto.getResourceId())) { issues.add(dto.toDefaultIssue()); ruleIds.add(dto.getRuleId()); } } - if (!issues.isEmpty()) { - return new DefaultResults(issues, getRulesByIssue(issues, ruleIds), getComponentsByIssue(issues, componentIds)); - } else { - return new DefaultResults(issues); - } + + return new DefaultResults(issues, getRulesByIssue(issues, ruleIds), getComponentsByIssue(issues, componentIds), pagination); } finally { MyBatis.closeQuietly(sqlSession); } } - private Set<Long> getAutorizedIssueIds(List<IssueDto> allIssuesDto, Set<Integer> authorizedComponentIds, IssueQuery query){ + private Set<Integer> getResourceIds(List<IssueDto> allIssuesDto) { + Set<Integer> componentIds = Sets.newLinkedHashSet(); + for (IssueDto issueDto : allIssuesDto) { + componentIds.add(issueDto.getResourceId()); + } + return componentIds; + } + + private List<IssueDto> getAuthorizedIssues(List<IssueDto> allIssuesDto, final Set<Integer> authorizedComponentIds) { + return newArrayList(Iterables.filter(allIssuesDto, new Predicate<IssueDto>() { + @Override + public boolean apply(IssueDto issueDto) { + return authorizedComponentIds.contains(issueDto.getResourceId()); + } + })); + } + + private Set<Long> getPaginatedAuthorizedIssueIds(List<IssueDto> authorizedIssues, Pagination pagination) { Set<Long> issueIds = Sets.newLinkedHashSet(); int index = 0; - for (IssueDto issueDto : allIssuesDto) { - if (authorizedComponentIds.contains(issueDto.getResourceId())) { - if (index >= query.offset() && issueIds.size() < query.limit()) { - issueIds.add(issueDto.getId()); - } else if (issueIds.size() >= query.limit()) { - break; - } + for (IssueDto issueDto : authorizedIssues) { + if (index >= pagination.offset() && issueIds.size() < pagination.limit()) { + issueIds.add(issueDto.getId()); + } else if (issueIds.size() >= pagination.limit()) { + break; } index++; } @@ -158,20 +169,15 @@ public class ServerIssueFinder implements IssueFinder { static class DefaultResults implements Results { private final List<Issue> issues; + private final Pagination pagination; private final Map<Issue, Rule> rulesByIssue; private final Map<Issue, Component> componentsByIssue; - // TODO add nb total issues and maybe some pagination methods - DefaultResults(List<Issue> issues, Map<Issue, Rule> rulesByIssue, Map<Issue, Component> componentsByIssue) { + DefaultResults(List<Issue> issues, Map<Issue, Rule> rulesByIssue, Map<Issue, Component> componentsByIssue, Pagination pagination) { this.issues = issues; this.rulesByIssue = rulesByIssue; this.componentsByIssue = componentsByIssue; - } - - DefaultResults(List<Issue> issues) { - this.issues = issues; - this.rulesByIssue = newHashMap(); - this.componentsByIssue = newHashMap(); + this.pagination = pagination; } @Override @@ -195,5 +201,8 @@ public class ServerIssueFinder implements IssueFinder { return componentsByIssue.values(); } + public Pagination pagination() { + return pagination; + } } } diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java index c043ebe1208..9a72ad109fd 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java @@ -113,7 +113,33 @@ public class ServerIssueFinderTest { IssueFinder.Results results = finder.find(issueQuery, null, UserRole.USER); verify(issueDao).selectByIds(eq(newHashSet(1L)), any(SqlSession.class)); - assertThat(results.issues()).hasSize(1); + } + + @Test + public void should_find_paginate_result() { + grantAccessRights(); + + IssueQuery issueQuery = mock(IssueQuery.class); + when(issueQuery.limit()).thenReturn(1); + when(issueQuery.page()).thenReturn(1); + + IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setResourceId(123) + .setComponentKey_unit_test_only("Action.java") + .setRuleKey_unit_test_only("squid", "AvoidCycle"); + IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setResourceId(135) + .setComponentKey_unit_test_only("Phases.java") + .setRuleKey_unit_test_only("squid", "AvoidCycle"); + List<IssueDto> dtoList = newArrayList(issue1, issue2); + when(issueDao.selectIssueIdsAndComponentsId(eq(issueQuery), any(SqlSession.class))).thenReturn(dtoList); + when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); + + IssueFinder.Results results = finder.find(issueQuery, null, UserRole.USER); + assertThat(results.pagination().offset()).isEqualTo(0); + assertThat(results.pagination().count()).isEqualTo(2); + assertThat(results.pagination().pages()).isEqualTo(2); + + // Only one result is expected because the limit is 1 + verify(issueDao).selectByIds(eq(newHashSet(1L)), any(SqlSession.class)); } @Test @@ -194,9 +220,6 @@ public class ServerIssueFinderTest { assertThat(results.issues()).isEmpty(); assertThat(results.rules()).isEmpty(); assertThat(results.components()).isEmpty(); - - verifyZeroInteractions(ruleFinder); - verifyZeroInteractions(resourceDao); } private void grantAccessRights() { |