From 40f40f2432bcf0874f264d605ef210d05c89bc74 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 24 Jul 2015 10:32:34 +0200 Subject: Improve some DAOs with conventions and DatabaseUtils.executeLargeInputs() --- .../org/sonar/core/user/DefaultUserFinder.java | 2 +- sonar-db/src/main/java/org/sonar/db/DaoModule.java | 2 + .../src/main/java/org/sonar/db/DatabaseUtils.java | 3 +- sonar-db/src/main/java/org/sonar/db/DbClient.java | 7 +++ .../java/org/sonar/db/issue/ActionPlanDao.java | 38 ++++++++------ .../java/org/sonar/db/issue/IssueChangeDao.java | 50 ++++++++++++------- .../src/main/java/org/sonar/db/issue/IssueDao.java | 6 +-- .../src/main/java/org/sonar/db/issue/IssueDto.java | 3 ++ .../src/main/java/org/sonar/db/rule/RuleDao.java | 58 ++++++++++++++++++++++ .../main/java/org/sonar/db/rule/RuleMapper.java | 2 + .../src/main/java/org/sonar/db/user/UserDao.java | 38 ++++++++------ .../main/java/org/sonar/db/user/UserMapper.java | 3 ++ .../resources/org/sonar/db/rule/RuleMapper.xml | 10 ++++ .../resources/org/sonar/db/user/UserMapper.xml | 10 ++++ .../org/sonar/core/user/DefaultUserFinderTest.java | 2 +- .../src/test/java/org/sonar/db/DaoModuleTest.java | 2 +- .../test/java/org/sonar/db/issue/IssueDaoTest.java | 4 +- .../test/java/org/sonar/db/rule/RuleDaoTest.java | 58 ++++++++++++++++++++++ .../test/java/org/sonar/db/user/UserDaoTest.java | 5 +- .../org/sonar/db/rule/RuleDaoTest/shared.xml | 12 +++++ 20 files changed, 256 insertions(+), 59 deletions(-) create mode 100644 sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java create mode 100644 sonar-db/src/test/java/org/sonar/db/rule/RuleDaoTest.java create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/shared.xml (limited to 'sonar-db') diff --git a/sonar-db/src/main/java/org/sonar/core/user/DefaultUserFinder.java b/sonar-db/src/main/java/org/sonar/core/user/DefaultUserFinder.java index 16d2867c6c0..428d7b27918 100644 --- a/sonar-db/src/main/java/org/sonar/core/user/DefaultUserFinder.java +++ b/sonar-db/src/main/java/org/sonar/core/user/DefaultUserFinder.java @@ -49,7 +49,7 @@ public class DefaultUserFinder implements UserFinder { @Override public List findByLogins(List logins) { - List dtos = userDao.selectUsersByLogins(logins); + List dtos = userDao.selectByLogins(logins); return toUsers(dtos); } diff --git a/sonar-db/src/main/java/org/sonar/db/DaoModule.java b/sonar-db/src/main/java/org/sonar/db/DaoModule.java index 6300c253adf..dde96831544 100644 --- a/sonar-db/src/main/java/org/sonar/db/DaoModule.java +++ b/sonar-db/src/main/java/org/sonar/db/DaoModule.java @@ -58,6 +58,7 @@ import org.sonar.db.qualitygate.ProjectQgateAssociationDao; import org.sonar.db.qualitygate.QualityGateConditionDao; import org.sonar.db.qualitygate.QualityGateDao; import org.sonar.db.qualityprofile.QualityProfileDao; +import org.sonar.db.rule.RuleDao; import org.sonar.db.source.FileSourceDao; import org.sonar.db.user.AuthorDao; import org.sonar.db.user.AuthorizationDao; @@ -102,6 +103,7 @@ public class DaoModule extends Module { ProjectQgateAssociationDao.class, QualityProfileDao.class, PurgeDao.class, + RuleDao.class, CharacteristicDao.class, ResourceIndexDao.class, ResourceDao.class, diff --git a/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java b/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java index a42de7dcafd..df9fbc98739 100644 --- a/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java +++ b/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java @@ -26,6 +26,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.sql.Timestamp; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Date; @@ -84,7 +85,7 @@ public class DatabaseUtils { if (input.isEmpty()) { return Collections.emptyList(); } - List results = newArrayList(); + List results = new ArrayList<>(); List> partitionList = Lists.partition(newArrayList(input), PARTITION_SIZE_FOR_ORACLE); for (List partition : partitionList) { List subResults = function.apply(partition); diff --git a/sonar-db/src/main/java/org/sonar/db/DbClient.java b/sonar-db/src/main/java/org/sonar/db/DbClient.java index 6ff2e99fdd6..f6471e7388e 100644 --- a/sonar-db/src/main/java/org/sonar/db/DbClient.java +++ b/sonar-db/src/main/java/org/sonar/db/DbClient.java @@ -57,6 +57,7 @@ import org.sonar.db.qualitygate.ProjectQgateAssociationDao; import org.sonar.db.qualitygate.QualityGateConditionDao; import org.sonar.db.qualitygate.QualityGateDao; import org.sonar.db.qualityprofile.QualityProfileDao; +import org.sonar.db.rule.RuleDao; import org.sonar.db.source.FileSourceDao; import org.sonar.db.user.AuthorDao; import org.sonar.db.user.AuthorizationDao; @@ -113,6 +114,7 @@ public class DbClient { private final CustomMeasureDao customMeasureDao; private final MetricDao metricDao; private final GroupDao groupDao; + private final RuleDao ruleDao; public DbClient(Database database, MyBatis myBatis, Dao... daos) { this.database = database; @@ -165,6 +167,7 @@ public class DbClient { customMeasureDao = getDao(map, CustomMeasureDao.class); metricDao = getDao(map, MetricDao.class); groupDao = getDao(map, GroupDao.class); + ruleDao = getDao(map, RuleDao.class); doOnLoad(map); } @@ -357,6 +360,10 @@ public class DbClient { return groupDao; } + public RuleDao ruleDao() { + return ruleDao; + } + protected K getDao(Map map, Class clazz) { return (K) map.get(clazz); } diff --git a/sonar-db/src/main/java/org/sonar/db/issue/ActionPlanDao.java b/sonar-db/src/main/java/org/sonar/db/issue/ActionPlanDao.java index b925c34b6ae..fed8d421950 100644 --- a/sonar-db/src/main/java/org/sonar/db/issue/ActionPlanDao.java +++ b/sonar-db/src/main/java/org/sonar/db/issue/ActionPlanDao.java @@ -20,16 +20,16 @@ package org.sonar.db.issue; -import com.google.common.collect.Lists; +import com.google.common.base.Function; import java.util.Collection; -import java.util.Collections; import java.util.List; +import javax.annotation.Nonnull; import org.apache.ibatis.session.SqlSession; import org.sonar.db.Dao; +import org.sonar.db.DatabaseUtils; +import org.sonar.db.DbSession; import org.sonar.db.MyBatis; -import static com.google.common.collect.Lists.newArrayList; - public class ActionPlanDao implements Dao { private final MyBatis mybatis; @@ -78,23 +78,31 @@ public class ActionPlanDao implements Dao { } public List selectByKeys(Collection keys) { - if (keys.isEmpty()) { - return Collections.emptyList(); - } - SqlSession session = mybatis.openSession(false); + DbSession session = mybatis.openSession(false); try { - List dtosList = newArrayList(); - List> keysPartition = Lists.partition(newArrayList(keys), 1000); - for (List partition : keysPartition) { - List dtos = session.getMapper(ActionPlanMapper.class).findByKeys(partition); - dtosList.addAll(dtos); - } - return dtosList; + return selectByKeys(session, keys); } finally { MyBatis.closeQuietly(session); } } + public List selectByKeys(DbSession dbSession, Collection keys) { + return DatabaseUtils.executeLargeInputs(keys, new SelectByKeys(dbSession.getMapper(ActionPlanMapper.class))); + } + + private static class SelectByKeys implements Function, List> { + private final ActionPlanMapper mapper; + + private SelectByKeys(ActionPlanMapper mapper) { + this.mapper = mapper; + } + + @Override + public List apply(@Nonnull List partitionOfKeys) { + return mapper.findByKeys(partitionOfKeys); + } + } + public List selectOpenByProjectId(Long projectId) { SqlSession session = mybatis.openSession(false); try { diff --git a/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java b/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java index 076681365eb..7a3f7bc686d 100644 --- a/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java +++ b/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java @@ -20,19 +20,20 @@ package org.sonar.db.issue; +import com.google.common.base.Function; import com.google.common.collect.Lists; import java.util.Collection; -import java.util.Collections; import java.util.List; import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import org.sonar.api.server.ServerSide; import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.FieldDiffs; import org.sonar.db.Dao; +import org.sonar.db.DatabaseUtils; import org.sonar.db.DbSession; import org.sonar.db.MyBatis; -import static com.google.common.collect.Lists.newArrayList; import static java.util.Arrays.asList; @ServerSide @@ -46,7 +47,7 @@ public class IssueChangeDao implements Dao { public List selectCommentsByIssues(DbSession session, Collection issueKeys) { List comments = Lists.newArrayList(); - for (IssueChangeDto dto : selectByIssuesAndType(session, issueKeys, IssueChangeDto.TYPE_COMMENT)) { + for (IssueChangeDto dto : selectByTypeAndIssueKeys(session, issueKeys, IssueChangeDto.TYPE_COMMENT)) { comments.add(dto.toComment()); } return comments; @@ -56,7 +57,7 @@ public class IssueChangeDao implements Dao { DbSession session = mybatis.openSession(false); try { List result = Lists.newArrayList(); - for (IssueChangeDto dto : selectByIssuesAndType(session, asList(issueKey), IssueChangeDto.TYPE_FIELD_CHANGE)) { + for (IssueChangeDto dto : selectByTypeAndIssueKeys(session, asList(issueKey), IssueChangeDto.TYPE_FIELD_CHANGE)) { result.add(dto.toFieldDiffs()); } return result; @@ -68,7 +69,7 @@ public class IssueChangeDao implements Dao { public List selectChangelogOfNonClosedIssuesByComponent(String componentUuid) { DbSession session = mybatis.openSession(false); try { - IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); + IssueChangeMapper mapper = mapper(session); return mapper.selectChangelogOfNonClosedIssuesByComponent(componentUuid, IssueChangeDto.TYPE_FIELD_CHANGE); } finally { @@ -80,7 +81,7 @@ public class IssueChangeDao implements Dao { public DefaultIssueComment selectCommentByKey(String commentKey) { DbSession session = mybatis.openSession(false); try { - IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); + IssueChangeMapper mapper = mapper(session); IssueChangeDto dto = mapper.selectByKeyAndType(commentKey, IssueChangeDto.TYPE_COMMENT); return dto != null ? dto.toComment() : null; @@ -89,28 +90,35 @@ public class IssueChangeDao implements Dao { } } - List selectByIssuesAndType(DbSession session, Collection issueKeys, String changeType) { - if (issueKeys.isEmpty()) { - return Collections.emptyList(); + public List selectByTypeAndIssueKeys(DbSession session, Collection issueKeys, String changeType) { + return DatabaseUtils.executeLargeInputs(issueKeys, new SelectByIssueKeys(mapper(session), changeType)); + } + + private static class SelectByIssueKeys implements Function, List> { + + private final IssueChangeMapper mapper; + private final String changeType; + + private SelectByIssueKeys(IssueChangeMapper mapper, String changeType) { + this.mapper = mapper; + this.changeType = changeType; } - IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); - List dtosList = newArrayList(); - List> keysPartition = Lists.partition(newArrayList(issueKeys), 1000); - for (List partition : keysPartition) { - List dtos = mapper.selectByIssuesAndType(partition, changeType); - dtosList.addAll(dtos); + + @Override + public List apply(@Nonnull List issueKeys) { + return mapper.selectByIssuesAndType(issueKeys, changeType); } - return dtosList; + } public void insert(DbSession session, IssueChangeDto change) { - session.getMapper(IssueChangeMapper.class).insert(change); + mapper(session).insert(change); } public boolean delete(String key) { DbSession session = mybatis.openSession(false); try { - IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); + IssueChangeMapper mapper = mapper(session); int count = mapper.delete(key); session.commit(); return count == 1; @@ -123,7 +131,7 @@ public class IssueChangeDao implements Dao { public boolean update(IssueChangeDto change) { DbSession session = mybatis.openSession(false); try { - IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); + IssueChangeMapper mapper = mapper(session); int count = mapper.update(change); session.commit(); return count == 1; @@ -132,4 +140,8 @@ public class IssueChangeDao implements Dao { MyBatis.closeQuietly(session); } } + + private IssueChangeMapper mapper(DbSession session) { + return session.getMapper(IssueChangeMapper.class); + } } diff --git a/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java b/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java index 42583413a09..36dbf668650 100644 --- a/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java +++ b/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java @@ -62,7 +62,7 @@ public class IssueDao implements Dao { return Optional.fromNullable(mapper(session).selectByKey(key)); } - public IssueDto selectByKeyOrFail(DbSession session, String key) { + public IssueDto selectOrFailByKey(DbSession session, String key) { Optional issue = selectByKey(session, key); if (!issue.isPresent()) { throw new RowNotFoundException(String.format("Issue with key '%s' does not exist", key)); @@ -103,9 +103,9 @@ public class IssueDao implements Dao { * if input keys contain multiple occurrences of a key. *

Contrary to {@link #selectByKeys(DbSession, List)}, results are in the same order as input keys.

*/ - public Iterable selectByOrderedKeys(DbSession session, List keys) { + public List selectByOrderedKeys(DbSession session, List keys) { List unordered = selectByKeys(session, keys); - return from(keys).transform(new KeyToIssue(unordered)).filter(Predicates.notNull()); + return from(keys).transform(new KeyToIssue(unordered)).filter(Predicates.notNull()).toList(); } private static class KeyToIssue implements Function { diff --git a/sonar-db/src/main/java/org/sonar/db/issue/IssueDto.java b/sonar-db/src/main/java/org/sonar/db/issue/IssueDto.java index ede94b1f5e2..51518fb1dc9 100644 --- a/sonar-db/src/main/java/org/sonar/db/issue/IssueDto.java +++ b/sonar-db/src/main/java/org/sonar/db/issue/IssueDto.java @@ -341,6 +341,7 @@ public final class IssueDto implements Serializable { return this; } + @CheckForNull public String getAssignee() { return assignee; } @@ -350,6 +351,7 @@ public final class IssueDto implements Serializable { return this; } + @CheckForNull public String getAuthorLogin() { return authorLogin; } @@ -359,6 +361,7 @@ public final class IssueDto implements Serializable { return this; } + @CheckForNull public String getIssueAttributes() { return issueAttributes; } diff --git a/sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java b/sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java new file mode 100644 index 00000000000..b20552a3b62 --- /dev/null +++ b/sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java @@ -0,0 +1,58 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 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.db.rule; + +import com.google.common.base.Function; +import java.util.List; +import javax.annotation.Nonnull; +import org.sonar.api.rule.RuleKey; +import org.sonar.db.Dao; +import org.sonar.db.DbSession; + +import static org.sonar.db.DatabaseUtils.executeLargeInputs; + +public class RuleDao implements Dao { + + /** + * Select rules by keys, whatever their status. Returns an empty list + * if the list of {@code keys} is empty, without any db round trip. + */ + public List selectByKeys(final DbSession session, List keys) { + return executeLargeInputs(keys, new KeyToDto(mapper(session))); + } + + private RuleMapper mapper(DbSession session) { + return session.getMapper(RuleMapper.class); + } + + private static class KeyToDto implements Function, List> { + private final RuleMapper mapper; + + private KeyToDto(RuleMapper mapper) { + this.mapper = mapper; + } + + @Override + public List apply(@Nonnull List partitionOfKeys) { + return mapper.selectByKeys(partitionOfKeys); + } + } + +} diff --git a/sonar-db/src/main/java/org/sonar/db/rule/RuleMapper.java b/sonar-db/src/main/java/org/sonar/db/rule/RuleMapper.java index 5074160a11c..79a1eebfe42 100644 --- a/sonar-db/src/main/java/org/sonar/db/rule/RuleMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/rule/RuleMapper.java @@ -39,6 +39,8 @@ public interface RuleMapper { RuleDto selectByKey(RuleKey ruleKey); + List selectByKeys(@Param("ruleKeys") List keys); + RuleDto selectByName(String name); void update(RuleDto rule); diff --git a/sonar-db/src/main/java/org/sonar/db/user/UserDao.java b/sonar-db/src/main/java/org/sonar/db/user/UserDao.java index 108fa563892..5c1e52d5f8c 100644 --- a/sonar-db/src/main/java/org/sonar/db/user/UserDao.java +++ b/sonar-db/src/main/java/org/sonar/db/user/UserDao.java @@ -19,13 +19,16 @@ */ package org.sonar.db.user; -import com.google.common.collect.Lists; +import com.google.common.base.Function; +import java.util.Collection; import java.util.List; import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import org.apache.ibatis.session.SqlSession; import org.sonar.api.user.UserQuery; import org.sonar.api.utils.System2; import org.sonar.db.Dao; +import org.sonar.db.DatabaseUtils; import org.sonar.db.DbSession; import org.sonar.db.MyBatis; import org.sonar.db.RowNotFoundException; @@ -74,27 +77,34 @@ public class UserDao implements Dao { return mapper.selectUserByLogin(login); } - public List selectUsersByLogins(List logins) { - List users = Lists.newArrayList(); + /** + * Select users by logins, including disabled users. An empty list is returned + * if list of logins is empty, without any db round trips. + */ + public List selectByLogins(DbSession session, Collection logins) { + return DatabaseUtils.executeLargeInputs(logins, new SelectByLogins(mapper(session))); + } + + public List selectByLogins(Collection logins) { DbSession session = mybatis.openSession(false); try { - users.addAll(selectUsersByLogins(session, logins)); + return selectByLogins(session, logins); } finally { MyBatis.closeQuietly(session); } - return users; } - public List selectUsersByLogins(DbSession session, List logins) { - List users = Lists.newArrayList(); - if (!logins.isEmpty()) { - UserMapper mapper = session.getMapper(UserMapper.class); - List> partitions = Lists.partition(logins, 1000); - for (List partition : partitions) { - users.addAll(mapper.selectUsersByLogins(partition)); - } + private static class SelectByLogins implements Function, List> { + private final UserMapper mapper; + + private SelectByLogins(UserMapper mapper) { + this.mapper = mapper; + } + + @Override + public List apply(@Nonnull List partitionOfLogins) { + return mapper.selectByLogins(partitionOfLogins); } - return users; } public List selectUsers(UserQuery query) { diff --git a/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java b/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java index e387210942a..bef663e87d1 100644 --- a/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java @@ -49,6 +49,8 @@ public interface UserMapper { List selectUsers(UserQuery query); + List selectByLogins(List logins); + @CheckForNull GroupDto selectGroupByName(String name); @@ -76,4 +78,5 @@ public interface UserMapper { void deactivateUser(@Param("id") long userId, @Param("now") long now); + } diff --git a/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml b/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml index fd93ba7b0a5..4651984855a 100644 --- a/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml @@ -71,6 +71,16 @@ FROM rules r WHERE r.plugin_name=#{repository} AND r.plugin_rule_key=#{rule} + + + +