From dc5ecf80639df3951adf3925df4b1f95e502db5b Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 12 Apr 2013 14:12:23 +0200 Subject: [PATCH] SONAR-3755 refactor IssueFinder --- .../sonar/core/issue/DefaultIssueFinder.java | 119 +++++++++++----- .../java/org/sonar/core/issue/IssueDao.java | 29 ++-- .../org/sonar/core/issue/IssueFilter.java | 101 ------------- .../org/sonar/core/issue/IssueMapper.java | 42 ------ .../org/sonar/core/persistence/MyBatis.java | 65 ++++++--- .../org/sonar/core/issue/IssueMapper.xml | 15 +- .../core/issue/DefaultIssueFinderTest.java | 24 ++-- .../org/sonar/core/issue/IssueDaoTest.java | 48 +++---- .../org/sonar/core/issue/IssueFilterTest.java | 120 ---------------- .../java/org/sonar/api/issue/IssueFinder.java | 9 +- .../java/org/sonar/api/issue/IssueQuery.java | 133 +++++++++--------- .../org/sonar/api/issue/IssueQueryTest.java | 56 ++++++++ .../org/sonar/server/platform/Platform.java | 42 +++++- .../java/org/sonar/server/ui/JRubyFacade.java | 7 +- 14 files changed, 358 insertions(+), 452 deletions(-) delete mode 100644 sonar-core/src/main/java/org/sonar/core/issue/IssueFilter.java delete mode 100644 sonar-core/src/main/java/org/sonar/core/issue/IssueMapper.java delete mode 100644 sonar-core/src/test/java/org/sonar/core/issue/IssueFilterTest.java create mode 100644 sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java index 3b4c833ef1c..5cd06de4759 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java @@ -21,7 +21,10 @@ package org.sonar.core.issue; import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.issue.Issue; @@ -32,10 +35,9 @@ import org.sonar.api.rules.RuleFinder; import org.sonar.core.resource.ResourceDao; import org.sonar.core.resource.ResourceDto; -import java.util.Collection; import java.util.List; - -import static com.google.common.collect.Lists.newArrayList; +import java.util.Map; +import java.util.Set; /** * @since 3.6 @@ -54,49 +56,92 @@ public class DefaultIssueFinder implements IssueFinder { this.ruleFinder = ruleFinder; } - public List find(IssueQuery issueQuery) { - LOG.debug("IssueQuery : {}", issueQuery); - Collection dtoList = issueDao.select(issueQuery); - return newArrayList(Iterables.transform(dtoList, new Function() { + public Results find(IssueQuery query) { + LOG.debug("IssueQuery : {}", query); + List dtoList = issueDao.select(query); + + final Set componentIds = Sets.newLinkedHashSet(); + final Set ruleIds = Sets.newLinkedHashSet(); + for (IssueDto dto : dtoList) { + componentIds.add(dto.getResourceId()); + ruleIds.add(dto.getRuleId()); + } + final Map rules = Maps.newHashMap(); + for (Integer ruleId : ruleIds) { + Rule rule = ruleFinder.findById(ruleId); + if (rule != null) { + rules.put(rule.getId(), rule); + } + } + final Map resources = Maps.newHashMap(); + for (Integer componentId : componentIds) { + ResourceDto resource = resourceDao.getResource(componentId); + if (resource != null) { + resources.put(resource.getId().intValue(), resource); + } + } + + // TODO verify authorization + + List issues = ImmutableList.copyOf(Iterables.transform(dtoList, new Function() { @Override - public Issue apply(IssueDto input) { - return toIssue(input); + public Issue apply(IssueDto dto) { + Rule rule = rules.get(dto.getRuleId()); + ResourceDto resource = resources.get(dto.getResourceId()); + return toIssue(dto, rule, resource); } })); + + + return new DefaultResults(issues); } - public Issue findByKey(String key){ - IssueDto issueDto = issueDao.findByUuid(key); - return issueDto != null ? toIssue(issueDto) : null; + public Issue findByKey(String key) { + IssueDto dto = issueDao.selectByKey(key); + Issue issue = null; + if (dto != null) { + Rule rule = ruleFinder.findById(dto.getRuleId()); + ResourceDto resource = resourceDao.getResource(dto.getResourceId()); + issue = toIssue(dto, rule, resource); + } + return issue; } - private Issue toIssue(IssueDto issueDto){ + private Issue toIssue(IssueDto dto, Rule rule, ResourceDto resource) { DefaultIssue issue = new DefaultIssue(); - issue.setKey(issueDto.getUuid()); - issue.setStatus(issueDto.getStatus()); - issue.setResolution(issueDto.getResolution()); - issue.setMessage(issueDto.getMessage()); - issue.setTitle(issueDto.getTitle()); - issue.setCost(issueDto.getCost()); - issue.setLine(issueDto.getLine()); - issue.setSeverity(issueDto.getSeverity()); - issue.setUserLogin(issueDto.getUserLogin()); - issue.setAssigneeLogin(issueDto.getAssigneeLogin()); - - // FIXME - ResourceDto resource = resourceDao.getResource(issueDto.getResourceId()); - issue.setComponentKey(resource.getKey()); - - // FIXME - Rule rule = ruleFinder.findById(issueDto.getRuleId()); - issue.setRuleKey(rule.getKey()); - issue.setRuleRepositoryKey(rule.getRepositoryKey()); - - issue.setCreatedAt(issueDto.getCreatedAt()); - issue.setUpdatedAt(issueDto.getCreatedAt()); - issue.setClosedAt(issueDto.getUpdatedAt()); - + issue.setKey(dto.getUuid()); + issue.setStatus(dto.getStatus()); + issue.setResolution(dto.getResolution()); + issue.setMessage(dto.getMessage()); + issue.setTitle(dto.getTitle()); + issue.setCost(dto.getCost()); + issue.setLine(dto.getLine()); + issue.setSeverity(dto.getSeverity()); + issue.setUserLogin(dto.getUserLogin()); + issue.setAssigneeLogin(dto.getAssigneeLogin()); + issue.setCreatedAt(dto.getCreatedAt()); + issue.setUpdatedAt(dto.getCreatedAt()); + issue.setClosedAt(dto.getUpdatedAt()); + if (resource != null) { + issue.setComponentKey(resource.getKey()); + } + if (rule != null) { + issue.setRuleKey(rule.getKey()); + issue.setRuleRepositoryKey(rule.getRepositoryKey()); + } return issue; } + static class DefaultResults implements Results { + private final List issues; + + DefaultResults(List issues) { + this.issues = issues; + } + + @Override + public List issues() { + return issues; + } + } } 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 a7523763ec4..8514ce2f75e 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 @@ -20,7 +20,7 @@ package org.sonar.core.issue; -import com.google.common.base.Preconditions; +import org.apache.ibatis.session.RowBounds; import org.apache.ibatis.session.SqlSession; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; @@ -28,6 +28,7 @@ import org.sonar.api.issue.IssueQuery; import org.sonar.core.persistence.MyBatis; import java.util.Collection; +import java.util.List; /** * @since 3.6 @@ -42,9 +43,9 @@ public class IssueDao implements BatchComponent, ServerComponent { public void insert(IssueDto issueDto) { SqlSession session = mybatis.openSession(); - IssueMapper mapper = session.getMapper(IssueMapper.class); try { - mapper.insert(issueDto); + // TODO bulk insert + session.insert("org.sonar.core.issue.IssueMapper.insert", issueDto); session.commit(); } finally { MyBatis.closeQuietly(session); @@ -52,13 +53,11 @@ public class IssueDao implements BatchComponent, ServerComponent { } public IssueDao update(Collection issues) { - Preconditions.checkNotNull(issues); - SqlSession session = mybatis.openBatchSession(); try { - IssueMapper mapper = session.getMapper(IssueMapper.class); + // TODO bulk update for (IssueDto issue : issues) { - mapper.update(issue); + session.update("org.sonar.core.issue.IssueMapper.update", issue); } session.commit(); return this; @@ -68,31 +67,29 @@ public class IssueDao implements BatchComponent, ServerComponent { } } - public IssueDto findById(long issueId) { + public IssueDto selectById(long id) { SqlSession session = mybatis.openSession(); try { - IssueMapper mapper = session.getMapper(IssueMapper.class); - return mapper.findById(issueId); + return session.selectOne("org.sonar.core.issue.IssueMapper.selectById", id); } finally { MyBatis.closeQuietly(session); } } - public IssueDto findByUuid(String uuid) { + public IssueDto selectByKey(String key) { SqlSession session = mybatis.openSession(); try { - IssueMapper mapper = session.getMapper(IssueMapper.class); - return mapper.findByUuid(uuid); + return session.selectOne("org.sonar.core.issue.IssueMapper.selectByKey", key); } finally { MyBatis.closeQuietly(session); } } - public Collection select(IssueQuery issueQuery) { + public List select(IssueQuery query) { SqlSession session = mybatis.openSession(); try { - IssueMapper mapper = session.getMapper(IssueMapper.class); - return mapper.select(issueQuery); + return session.selectList("org.sonar.core.issue.IssueMapper.select", query, new RowBounds(query.offset(), query.limit())); + } finally { MyBatis.closeQuietly(session); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueFilter.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueFilter.java deleted file mode 100644 index beb41817836..00000000000 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueFilter.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Sonar, open source software quality management tool. - * Copyright (C) 2008-2012 SonarSource - * mailto:contact AT sonarsource DOT com - * - * Sonar 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. - * - * Sonar 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 Sonar; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 - */ - -package org.sonar.core.issue; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Splitter; -import com.google.common.base.Strings; -import org.sonar.api.ServerComponent; -import org.sonar.api.issue.Issue; -import org.sonar.api.issue.IssueFinder; -import org.sonar.api.issue.IssueQuery; - -import java.util.List; -import java.util.Map; - -import static com.google.common.collect.Lists.newArrayList; - -/** - * @since 3.6 - */ -public class IssueFilter implements ServerComponent { - - private final IssueFinder issueFinder; - - public IssueFilter(IssueFinder issueFinder) { - this.issueFinder = issueFinder; - } - - public List execute(Map map) { - IssueQuery issueQueryBuilder = createIssueQuery(map); - return issueFinder.find(issueQueryBuilder); - } - - public Issue execute(String key) { - return !Strings.isNullOrEmpty(key) ? issueFinder.findByKey(key) : null; - } - - @VisibleForTesting - IssueQuery createIssueQuery(Map map) { - IssueQuery.Builder issueQueryBuilder = new IssueQuery.Builder(); - if (map != null && !map.isEmpty()) { - if (isPropertyNotEmpty("keys", map)) { - issueQueryBuilder.keys(getListProperties("keys", map)); - } - if (isPropertyNotEmpty("severities", map)) { - issueQueryBuilder.severities(getListProperties("severities", map)); - } - if (isPropertyNotEmpty("minSeverity", map)) { - issueQueryBuilder.minSeverity(map.get("minSeverity")); - } - if (isPropertyNotEmpty("status", map)) { - issueQueryBuilder.status(getListProperties("status", map)); - } - if (isPropertyNotEmpty("resolutions", map)) { - issueQueryBuilder.resolutions(getListProperties("resolutions", map)); - } - if (isPropertyNotEmpty("components", map)) { - issueQueryBuilder.componentKeys(getListProperties("components", map)); - } - if (isPropertyNotEmpty("rules", map)) { - issueQueryBuilder.rules(getListProperties("rules", map)); - } - if (isPropertyNotEmpty("userLogins", map)) { - issueQueryBuilder.userLogins(getListProperties("userLogins", map)); - } - if (isPropertyNotEmpty("assigneeLogins", map)) { - issueQueryBuilder.assigneeLogins(getListProperties("assigneeLogins", map)); - } - if (isPropertyNotEmpty("limit", map)) { - issueQueryBuilder.limit(Integer.parseInt(map.get("limit"))); - } - } - return issueQueryBuilder.build(); - } - - private boolean isPropertyNotEmpty(String property, Map map) { - return map.containsKey(property) && !Strings.isNullOrEmpty(map.get(property)); - } - - private List getListProperties(String property, Map map) { - return newArrayList(Splitter.on(',').split(map.get(property))); - } -} diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueMapper.java deleted file mode 100644 index 78891c3a34f..00000000000 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueMapper.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Sonar, open source software quality management tool. - * Copyright (C) 2008-2012 SonarSource - * mailto:contact AT sonarsource DOT com - * - * Sonar 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. - * - * Sonar 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 Sonar; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 - */ - -package org.sonar.core.issue; - -import org.sonar.api.issue.IssueQuery; - -import java.util.Collection; - -/** - * @since 3.6 - */ -public interface IssueMapper { - - void insert(IssueDto issueDto); - - void update(IssueDto review); - - IssueDto findById(long issueId); - - IssueDto findByUuid(String uuid); - - Collection select(IssueQuery issueQuery); - -} diff --git a/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java b/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java index bba58e1a82d..3155587c191 100644 --- a/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java +++ b/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java @@ -24,7 +24,11 @@ import com.google.common.io.Closeables; import org.apache.ibatis.builder.xml.XMLMapperBuilder; import org.apache.ibatis.logging.LogFactory; import org.apache.ibatis.mapping.Environment; -import org.apache.ibatis.session.*; +import org.apache.ibatis.session.Configuration; +import org.apache.ibatis.session.ExecutorType; +import org.apache.ibatis.session.SqlSession; +import org.apache.ibatis.session.SqlSessionFactory; +import org.apache.ibatis.session.SqlSessionFactoryBuilder; import org.apache.ibatis.transaction.jdbc.JdbcTransactionFactory; import org.apache.ibatis.type.JdbcType; import org.slf4j.LoggerFactory; @@ -35,7 +39,14 @@ import org.sonar.api.database.model.MeasureData; import org.sonar.api.database.model.MeasureMapper; import org.sonar.api.database.model.MeasureModel; import org.sonar.core.config.Logback; -import org.sonar.core.dashboard.*; +import org.sonar.core.dashboard.ActiveDashboardDto; +import org.sonar.core.dashboard.ActiveDashboardMapper; +import org.sonar.core.dashboard.DashboardDto; +import org.sonar.core.dashboard.DashboardMapper; +import org.sonar.core.dashboard.WidgetDto; +import org.sonar.core.dashboard.WidgetMapper; +import org.sonar.core.dashboard.WidgetPropertyDto; +import org.sonar.core.dashboard.WidgetPropertyMapper; import org.sonar.core.dependency.DependencyDto; import org.sonar.core.dependency.DependencyMapper; import org.sonar.core.dependency.ResourceSnapshotDto; @@ -47,14 +58,18 @@ import org.sonar.core.graph.jdbc.GraphDtoMapper; import org.sonar.core.issue.IssueChangeDto; import org.sonar.core.issue.IssueChangeMapper; import org.sonar.core.issue.IssueDto; -import org.sonar.core.issue.IssueMapper; import org.sonar.core.measure.MeasureFilterDto; import org.sonar.core.measure.MeasureFilterMapper; import org.sonar.core.properties.PropertiesMapper; import org.sonar.core.properties.PropertyDto; import org.sonar.core.purge.PurgeMapper; import org.sonar.core.purge.PurgeableSnapshotDto; -import org.sonar.core.resource.*; +import org.sonar.core.resource.ResourceDto; +import org.sonar.core.resource.ResourceIndexDto; +import org.sonar.core.resource.ResourceIndexerMapper; +import org.sonar.core.resource.ResourceKeyUpdaterMapper; +import org.sonar.core.resource.ResourceMapper; +import org.sonar.core.resource.SnapshotDto; import org.sonar.core.review.ReviewCommentDto; import org.sonar.core.review.ReviewCommentMapper; import org.sonar.core.review.ReviewDto; @@ -66,7 +81,14 @@ import org.sonar.core.source.jdbc.SnapshotDataMapper; import org.sonar.core.source.jdbc.SnapshotSourceMapper; import org.sonar.core.template.LoadedTemplateDto; import org.sonar.core.template.LoadedTemplateMapper; -import org.sonar.core.user.*; +import org.sonar.core.user.AuthorDto; +import org.sonar.core.user.AuthorMapper; +import org.sonar.core.user.GroupDto; +import org.sonar.core.user.GroupRoleDto; +import org.sonar.core.user.RoleMapper; +import org.sonar.core.user.UserDto; +import org.sonar.core.user.UserMapper; +import org.sonar.core.user.UserRoleDto; import java.io.InputStream; @@ -127,13 +149,14 @@ public class MyBatis implements BatchComponent, ServerComponent { loadAlias(conf, "SnapshotData", SnapshotDataDto.class); Class[] mappers = {ActiveDashboardMapper.class, AuthorMapper.class, DashboardMapper.class, - DependencyMapper.class, DuplicationMapper.class, GraphDtoMapper.class, IssueMapper.class, IssueChangeMapper.class, LoadedTemplateMapper.class, - MeasureFilterMapper.class, PropertiesMapper.class, PurgeMapper.class, ResourceKeyUpdaterMapper.class, ResourceIndexerMapper.class, ResourceMapper.class, - ResourceSnapshotMapper.class, ReviewCommentMapper.class, ReviewMapper.class, RoleMapper.class, RuleMapper.class, SchemaMigrationMapper.class, - SemaphoreMapper.class, UserMapper.class, WidgetMapper.class, WidgetPropertyMapper.class, MeasureMapper.class, SnapshotDataMapper.class, - SnapshotSourceMapper.class + DependencyMapper.class, DuplicationMapper.class, GraphDtoMapper.class, IssueChangeMapper.class, LoadedTemplateMapper.class, + MeasureFilterMapper.class, PropertiesMapper.class, PurgeMapper.class, ResourceKeyUpdaterMapper.class, ResourceIndexerMapper.class, ResourceMapper.class, + ResourceSnapshotMapper.class, ReviewCommentMapper.class, ReviewMapper.class, RoleMapper.class, RuleMapper.class, SchemaMigrationMapper.class, + SemaphoreMapper.class, UserMapper.class, WidgetMapper.class, WidgetPropertyMapper.class, MeasureMapper.class, SnapshotDataMapper.class, + SnapshotSourceMapper.class }; loadMappers(conf, mappers); + loadMapper(conf, "org.sonar.core.issue.IssueMapper"); configureLogback(mappers); sessionFactory = new SqlSessionFactoryBuilder().build(conf); @@ -184,18 +207,20 @@ public class MyBatis implements BatchComponent, ServerComponent { } private void loadMapper(Configuration configuration, Class mapperClass) { - String mapperName = mapperClass.getName(); - - InputStream input = null; - try { - input = getClass().getResourceAsStream("/" + mapperName.replace('.', '/') + ".xml"); - new XMLMapperBuilder(input, configuration, mapperName, configuration.getSqlFragments()).parse(); - configuration.addLoadedResource(mapperName); - } finally { - Closeables.closeQuietly(input); - } + loadMapper(configuration, mapperClass.getName()); } + private void loadMapper(Configuration configuration, String mapperName) { + InputStream input = null; + try { + input = getClass().getResourceAsStream("/" + mapperName.replace('.', '/') + ".xml"); + new XMLMapperBuilder(input, configuration, mapperName, configuration.getSqlFragments()).parse(); + configuration.addLoadedResource(mapperName); + } finally { + Closeables.closeQuietly(input); + } + } + private void loadAlias(Configuration conf, String alias, Class dtoClass) { conf.getTypeAliasRegistry().registerAlias(alias, dtoClass); } diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml index 2ecc8510db0..d0aa11215a6 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml @@ -71,13 +71,13 @@ where id = #{id} - select from issues i where i.id=#{id} - select from issues i where i.uuid=#{uuid} @@ -85,14 +85,15 @@