From 744a413b6c0f540117da5cc0ace02c72e81926f3 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 29 Jun 2017 10:42:55 +0200 Subject: SONAR-9482 Use query object to count active rules in db --- .../db/qualityprofile/ActiveRuleCountQuery.java | 104 +++++++++++++++++ .../org/sonar/db/qualityprofile/ActiveRuleDao.java | 25 +--- .../sonar/db/qualityprofile/ActiveRuleMapper.java | 7 +- .../sonar/db/qualityprofile/ActiveRuleMapper.xml | 43 +++---- .../sonar/db/qualityprofile/ActiveRuleDaoTest.java | 127 ++++++++------------- 5 files changed, 175 insertions(+), 131 deletions(-) create mode 100644 server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleCountQuery.java (limited to 'server/sonar-db-dao') diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleCountQuery.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleCountQuery.java new file mode 100644 index 00000000000..02303b9b8be --- /dev/null +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleCountQuery.java @@ -0,0 +1,104 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.qualityprofile; + +import java.util.List; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import org.sonar.api.rule.RuleStatus; +import org.sonar.core.util.stream.MoreCollectors; +import org.sonar.db.organization.OrganizationDto; + +import static com.google.common.base.Preconditions.checkState; + +public class ActiveRuleCountQuery { + + private final OrganizationDto organization; + private final List profileUuids; + private final RuleStatus ruleStatus; + private final String inheritance; + + public ActiveRuleCountQuery(Builder builder) { + this.profileUuids = builder.profiles.stream().map(QProfileDto::getKee).collect(MoreCollectors.toList()); + this.ruleStatus = builder.ruleStatus; + this.inheritance = builder.inheritance; + this.organization = builder.organization; + } + + public OrganizationDto getOrganization() { + return organization; + } + + public List getProfileUuids() { + return profileUuids; + } + + /** + * When no rule status is set, removed rules are not returned + */ + @CheckForNull + public RuleStatus getRuleStatus() { + return ruleStatus; + } + + @CheckForNull + public String getInheritance() { + return inheritance; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private OrganizationDto organization; + private List profiles; + private RuleStatus ruleStatus; + private String inheritance; + + public Builder setOrganization(OrganizationDto organization) { + this.organization = organization; + return this; + } + + public Builder setProfiles(List profiles) { + this.profiles = profiles; + return this; + } + + public Builder setRuleStatus(@Nullable RuleStatus ruleStatus) { + this.ruleStatus = ruleStatus; + return this; + } + + public Builder setInheritance(@Nullable String inheritance) { + this.inheritance = inheritance; + return this; + } + + public ActiveRuleCountQuery build() { + checkState(organization != null, "Organization cannot be null"); + checkState(profiles != null, "Profiles cannot be null"); + return new ActiveRuleCountQuery(this); + } + } + +} diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java index e62d881de6d..a699eab7870 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java @@ -24,15 +24,14 @@ import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; -import org.sonar.api.rule.RuleStatus; import org.sonar.db.Dao; import org.sonar.db.DatabaseUtils; import org.sonar.db.DbSession; -import org.sonar.db.KeyLongValue; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.rule.RuleParamDto; import static org.sonar.db.DatabaseUtils.executeLargeInputs; +import static org.sonar.db.KeyLongValue.toMap; public class ActiveRuleDao implements Dao { @@ -166,25 +165,9 @@ public class ActiveRuleDao implements Dao { DatabaseUtils.executeLargeUpdates(activeRuleIds, mapper::deleteParamsByActiveRuleIds); } - /** - * Active rule on removed rule are NOT taken into account - */ - public Map countActiveRulesByProfileUuid(DbSession dbSession, OrganizationDto organization) { - return KeyLongValue.toMap( - mapper(dbSession).countActiveRulesByProfileUuid(organization.getUuid())); - } - - public Map countActiveRulesForRuleStatusByProfileUuid(DbSession dbSession, OrganizationDto organization, RuleStatus ruleStatus) { - return KeyLongValue.toMap( - mapper(dbSession).countActiveRulesForRuleStatusByProfileUuid(organization.getUuid(), ruleStatus)); - } - - /** - * Active rule on removed rule are NOT taken into account - */ - public Map countActiveRulesForInheritanceByProfileUuid(DbSession dbSession, OrganizationDto organization, String inheritance) { - return KeyLongValue.toMap( - mapper(dbSession).countActiveRulesForInheritanceByProfileUuid(organization.getUuid(), inheritance)); + public Map countActiveRulesByQuery(DbSession dbSession, ActiveRuleCountQuery query) { + return toMap(executeLargeInputs(query.getProfileUuids(), + partition -> mapper(dbSession).countActiveRulesByQuery(query.getOrganization().getUuid(), partition, query.getRuleStatus(), query.getInheritance()))); } private static ActiveRuleMapper mapper(DbSession dbSession) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java index 5c384a20ccd..6a64c21ee4e 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java @@ -22,6 +22,7 @@ package org.sonar.db.qualityprofile; import java.util.Collection; import java.util.List; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import org.apache.ibatis.annotations.Param; import org.sonar.api.rule.RuleStatus; import org.sonar.db.KeyLongValue; @@ -69,9 +70,7 @@ public interface ActiveRuleMapper { List selectParamsByActiveRuleIds(@Param("ids") List ids); - List countActiveRulesByProfileUuid(@Param("organizationUuid") String organizationUuid); + List countActiveRulesByQuery(@Param("organizationUuid") String organizationUuid, @Param("profileUuids") List profileUuids, + @Nullable @Param("ruleStatus") RuleStatus ruleStatus, @Param("inheritance") String inheritance); - List countActiveRulesForRuleStatusByProfileUuid(@Param("organizationUuid") String organizationUuid, @Param("ruleStatus") RuleStatus ruleStatus); - - List countActiveRulesForInheritanceByProfileUuid(@Param("organizationUuid") String organizationUuid, @Param("inheritance") String inheritance); } diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml index 840467e7e8b..29ac1dc7481 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml @@ -246,38 +246,31 @@ - select oqp.uuid as "key", count(ar.id) as "value" from active_rules ar inner join rules_profiles rp on rp.id = ar.profile_id inner join org_qprofiles oqp on oqp.rules_profile_uuid = rp.kee inner join rules r on r.id = ar.rule_id - where oqp.organization_uuid = #{organizationUuid, jdbcType=VARCHAR} - and r.status != 'REMOVED' - group by oqp.uuid - - - - diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java index 62c4c989d77..8eb040b9d0b 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java @@ -19,15 +19,12 @@ */ package org.sonar.db.qualityprofile; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Objects; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; import org.sonar.api.server.rule.RuleParamType; import org.sonar.api.utils.System2; @@ -35,17 +32,19 @@ import org.sonar.api.utils.internal.TestSystem2; import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.organization.OrganizationDto; -import org.sonar.db.organization.OrganizationTesting; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleParamDto; -import org.sonar.db.rule.RuleTesting; import static com.google.common.collect.Lists.newArrayList; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.assertj.core.api.Assertions.tuple; -import static org.assertj.core.data.MapEntry.entry; +import static org.sonar.api.rule.RuleStatus.BETA; +import static org.sonar.api.rule.RuleStatus.READY; +import static org.sonar.api.rule.RuleStatus.REMOVED; import static org.sonar.api.rule.Severity.BLOCKER; import static org.sonar.api.rule.Severity.MAJOR; import static org.sonar.db.qualityprofile.ActiveRuleDto.INHERITED; @@ -86,7 +85,7 @@ public class ActiveRuleDaoTest { rule1 = db.rules().insert(); rule2 = db.rules().insert(); rule3 = db.rules().insert(); - removedRule = db.rules().insert(r -> r.setStatus(RuleStatus.REMOVED)); + removedRule = db.rules().insert(r -> r.setStatus(REMOVED)); rule1Param1 = new RuleParamDto() .setName("param1") @@ -128,7 +127,7 @@ public class ActiveRuleDaoTest { underTest.insert(dbSession, activeRule3); dbSession.commit(); - assertThat(underTest.selectByRuleIds(dbSession, organization, Collections.singletonList(rule1.getId()))) + assertThat(underTest.selectByRuleIds(dbSession, organization, singletonList(rule1.getId()))) .extracting("key").containsOnly(activeRule1.getKey(), activeRule3.getKey()); assertThat(underTest.selectByRuleIds(dbSession, organization, newArrayList(rule1.getId(), rule2.getId()))) .extracting("key").containsOnly(activeRule1.getKey(), activeRule2.getKey(), activeRule3.getKey()); @@ -516,101 +515,67 @@ public class ActiveRuleDaoTest { } @Test - public void test_countActiveRulesByProfileKey_for_a_specified_organization() { + public void countActiveRulesByQuery_filter_by_profiles() { db.qualityProfiles().activateRule(profile1, rule1); db.qualityProfiles().activateRule(profile1, rule2); + db.qualityProfiles().activateRule(profile1, removedRule); db.qualityProfiles().activateRule(profile2, rule1); + QProfileDto profileWithoutActiveRule = db.qualityProfiles().insert(organization); - Map counts = underTest.countActiveRulesByProfileUuid(dbSession, organization); - - assertThat(counts).containsOnly( + ActiveRuleCountQuery.Builder builder = ActiveRuleCountQuery.builder().setOrganization(organization); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1, profile2)).build())) + .containsOnly(entry(profile1.getKee(), 2L), entry(profile2.getKee(), 1L)); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(singletonList(profileWithoutActiveRule)).build())).isEmpty(); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1, profile2, profileWithoutActiveRule)).build())).containsOnly( entry(profile1.getKee(), 2L), entry(profile2.getKee(), 1L)); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(emptyList()).build())).isEmpty(); } @Test - public void countActiveRulesByProfileKey_returns_empty_map_if_organization_does_not_exist() { - Map counts = underTest.countActiveRulesByProfileUuid(dbSession, OrganizationTesting.newOrganizationDto()); - - assertThat(counts).isEmpty(); - } - - @Test - public void countActiveRulesByProfileKey_returns_empty_map_if_profile_does_not_have_active_rules() { - Map counts = underTest.countActiveRulesByProfileUuid(dbSession, organization); - - assertThat(counts).isEmpty(); - } - - @Test - public void countActiveRulesByProfileKey_ignores_removed_rules() { + public void countActiveRulesByQuery_filter_by_rule_status() { + RuleDefinitionDto betaRule = db.rules().insert(r -> r.setStatus(BETA)); db.qualityProfiles().activateRule(profile1, rule1); + db.qualityProfiles().activateRule(profile1, rule2); + db.qualityProfiles().activateRule(profile1, betaRule); db.qualityProfiles().activateRule(profile1, removedRule); + db.qualityProfiles().activateRule(profile2, rule1); + db.qualityProfiles().activateRule(profile2, betaRule); - Map counts = underTest.countActiveRulesByProfileUuid(dbSession, organization); - - assertThat(counts).containsExactly(entry(profile1.getKee(), 1L)); + ActiveRuleCountQuery.Builder builder = ActiveRuleCountQuery.builder().setOrganization(organization); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1, profile2)).setRuleStatus(BETA).build())) + .containsOnly(entry(profile1.getKee(), 1L), entry(profile2.getKee(), 1L)); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(singletonList(profile1)).setRuleStatus(READY).build())) + .containsOnly(entry(profile1.getKee(), 2L)); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(singletonList(profile1)).setRuleStatus(REMOVED).build())) + .containsOnly(entry(profile1.getKee(), 1L)); } @Test - public void test_countActiveRulesForRuleStatusByProfileKey_for_a_specified_organization() { - RuleDefinitionDto betaRule1 = db.rules().insertRule(RuleTesting.newRuleDto().setStatus(RuleStatus.BETA)).getDefinition(); - RuleDefinitionDto betaRule2 = db.rules().insertRule(RuleTesting.newRuleDto().setStatus(RuleStatus.BETA)).getDefinition(); + public void countActiveRulesByQuery_filter_by_inheritance() { db.qualityProfiles().activateRule(profile1, rule1); - db.qualityProfiles().activateRule(profile2, betaRule1); - db.qualityProfiles().activateRule(profile2, betaRule2); - - Map counts = underTest.countActiveRulesForRuleStatusByProfileUuid(dbSession, organization, RuleStatus.BETA); - - assertThat(counts).containsOnly(entry(profile2.getKee(), 2L)); - } - - @Test - public void countActiveRulesForRuleStatusByProfileKey_returns_empty_map_if_organization_does_not_exist() { - Map counts = underTest.countActiveRulesForRuleStatusByProfileUuid(dbSession, OrganizationTesting.newOrganizationDto(), RuleStatus.READY); + db.qualityProfiles().activateRule(profile1, rule2, ar -> ar.setInheritance(OVERRIDES)); + db.qualityProfiles().activateRule(profile1, removedRule, ar -> ar.setInheritance(OVERRIDES)); + db.qualityProfiles().activateRule(profile2, rule1, ar -> ar.setInheritance(OVERRIDES)); + db.qualityProfiles().activateRule(profile2, rule2, ar -> ar.setInheritance(INHERITED)); - assertThat(counts).isEmpty(); + ActiveRuleCountQuery.Builder builder = ActiveRuleCountQuery.builder().setOrganization(organization); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1, profile2)).setInheritance(OVERRIDES).build())) + .containsOnly(entry(profile1.getKee(), 1L), entry(profile2.getKee(), 1L)); + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1, profile2)).setInheritance(INHERITED).build())) + .containsOnly(entry(profile2.getKee(), 1L)); } @Test - public void countActiveRulesForRuleStatusByProfileKey_returns_empty_map_if_profile_does_not_have_rules_with_specified_status() { - Map counts = underTest.countActiveRulesForRuleStatusByProfileUuid(dbSession, organization, RuleStatus.DEPRECATED); - - assertThat(counts).isEmpty(); - } - - @Test - public void test_countActiveRulesForInheritanceByProfileKey_for_a_specified_organization() { + public void countActiveRulesByQuery_filter_by_organization() { db.qualityProfiles().activateRule(profile1, rule1); - db.qualityProfiles().activateRule(profile2, rule1, ar -> ar.setInheritance(ActiveRuleDto.OVERRIDES)); - db.qualityProfiles().activateRule(profile2, rule2, ar -> ar.setInheritance(ActiveRuleDto.INHERITED)); - - Map counts = underTest.countActiveRulesForInheritanceByProfileUuid(dbSession, organization, ActiveRuleDto.OVERRIDES); - - assertThat(counts).containsOnly(entry(profile2.getKee(), 1L)); - } - - @Test - public void countActiveRulesForInheritanceByProfileKey_returns_empty_map_if_organization_does_not_exist() { - Map counts = underTest.countActiveRulesForInheritanceByProfileUuid(dbSession, OrganizationTesting.newOrganizationDto(), ActiveRuleDto.OVERRIDES); + OrganizationDto anotherOrganization = db.organizations().insert(); + QProfileDto profileOnAnotherOrganization = db.qualityProfiles().insert(anotherOrganization); + db.qualityProfiles().activateRule(profileOnAnotherOrganization, rule1); - assertThat(counts).isEmpty(); + assertThat(underTest.countActiveRulesByQuery(dbSession, + ActiveRuleCountQuery.builder().setOrganization(organization).setProfiles(asList(profile1, profileOnAnotherOrganization)).build())) + .containsOnly(entry(profile1.getKee(), 1L)); } - @Test - public void countActiveRulesForInheritanceByProfileKey_returns_empty_map_if_profile_does_not_have_rules_with_specified_status() { - Map counts = underTest.countActiveRulesForInheritanceByProfileUuid(dbSession, organization, ActiveRuleDto.OVERRIDES); - - assertThat(counts).isEmpty(); - } - - @Test - public void countActiveRulesForInheritanceByProfileKey_ignores_removed_rules() { - db.qualityProfiles().activateRule(profile1, rule1, ar -> ar.setInheritance(ActiveRuleDto.OVERRIDES)); - db.qualityProfiles().activateRule(profile1, removedRule, ar -> ar.setInheritance(ActiveRuleDto.OVERRIDES)); - - Map counts = underTest.countActiveRulesForInheritanceByProfileUuid(dbSession, organization, ActiveRuleDto.OVERRIDES); - - assertThat(counts).containsOnly(entry(profile1.getKee(), 1L)); - } } -- cgit v1.2.3