From 247e661a3f0295f295c17567772b63aade9df020 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 26 Jan 2018 18:00:17 +0100 Subject: [PATCH] SONAR-10052 optimize data loading for (de)activation of rules --- .../db/qualityprofile/ActiveRuleDao.java | 32 +- .../db/qualityprofile/ActiveRuleMapper.java | 4 + .../db/qualityprofile/QualityProfileDao.java | 4 +- .../qualityprofile/QualityProfileMapper.java | 2 +- .../db/qualityprofile/ActiveRuleMapper.xml | 10 + .../qualityprofile/QualityProfileMapper.xml | 8 +- .../db/qualityprofile/ActiveRuleDaoTest.java | 70 ++- .../qualityprofile/QualityProfileDaoTest.java | 64 ++- .../platformlevel/PlatformLevel4.java | 6 +- .../BuiltInQProfileUpdateImpl.java | 27 +- .../qualityprofile/QProfileExporters.java | 43 +- .../qualityprofile/QProfileResetImpl.java | 19 +- .../server/qualityprofile/QProfileRules.java | 66 +++ .../qualityprofile/QProfileRulesImpl.java | 139 +++++ .../server/qualityprofile/QProfileTree.java | 37 ++ .../qualityprofile/QProfileTreeImpl.java | 141 +++++ .../RegisterQualityProfiles.java | 4 +- .../qualityprofile/RuleActivationContext.java | 335 ++++++++++++ .../server/qualityprofile/RuleActivator.java | 489 ++++++++---------- .../qualityprofile/RuleActivatorContext.java | 264 ---------- .../RuleActivatorContextFactory.java | 113 ---- .../qualityprofile/ws/ActivateRuleAction.java | 9 +- .../ws/ActivateRulesAction.java | 10 +- .../qualityprofile/ws/ChangeParentAction.java | 8 +- .../ws/DeactivateRuleAction.java | 9 +- .../ws/DeactivateRulesAction.java | 8 +- .../org/sonar/server/rule/RegisterRules.java | 14 +- .../sonar/server/rule/ws/DeleteAction.java | 14 +- .../BuiltInQProfileUpdateImplTest.java | 50 +- .../QProfileComparisonTest.java | 51 +- .../qualityprofile/QProfileExportersTest.java | 20 +- .../qualityprofile/QProfileResetImplTest.java | 14 +- ...torTest.java => QProfileRuleImplTest.java} | 253 ++------- .../qualityprofile/QProfileTesting.java | 3 +- .../qualityprofile/QProfileTreeImplTest.java | 272 ++++++++++ ...gisterQualityProfilesNotificationTest.java | 45 +- .../ws/ActivateRuleActionTest.java | 42 +- .../ws/ActivateRulesActionTest.java | 12 +- .../ws/ChangeParentActionTest.java | 26 +- .../qualityprofile/ws/CreateActionTest.java | 10 +- .../ws/DeactivateRuleActionTest.java | 24 +- .../ws/DeactivateRulesActionTest.java | 10 +- .../ws/InheritanceActionTest.java | 30 +- .../ws/QProfilesWsMediumTest.java | 25 +- .../sonar/server/rule/RegisterRulesTest.java | 6 +- .../server/rule/ws/DeleteActionTest.java | 6 +- .../server/rule/ws/SearchActionTest.java | 21 +- 47 files changed, 1681 insertions(+), 1188 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRules.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRulesImpl.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTree.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java delete mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java delete mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java rename server/sonar-server/src/test/java/org/sonar/server/qualityprofile/{RuleActivatorTest.java => QProfileRuleImplTest.java} (80%) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java 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 1b0acb1833a..1fe5c2c5a39 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 @@ -25,12 +25,17 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.Consumer; +import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.Dao; import org.sonar.db.DatabaseUtils; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleParamDto; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Collections.emptyList; +import static org.sonar.db.DatabaseUtils.PARTITION_SIZE_FOR_ORACLE; import static org.sonar.db.DatabaseUtils.executeLargeInputs; import static org.sonar.db.DatabaseUtils.executeLargeInputsWithoutOutput; import static org.sonar.db.KeyLongValue.toMap; @@ -76,18 +81,28 @@ public class ActiveRuleDao implements Dao { return mapper(dbSession).selectByRuleProfileUuid(ruleProfileDto.getKee()); } + public Collection selectByRulesAndRuleProfileUuids(DbSession dbSession, Collection rules, Collection ruleProfileUuids) { + if (rules.isEmpty()) { + return emptyList(); + } + checkArgument(rules.size() < PARTITION_SIZE_FOR_ORACLE, + "too many rules (got %s, max is %s)", rules.size(), PARTITION_SIZE_FOR_ORACLE); + List ruleIds = rules.stream().map(RuleDefinitionDto::getId).collect(MoreCollectors.toArrayList(rules.size())); + return executeLargeInputs(ruleProfileUuids, chunk -> mapper(dbSession).selectByRuleIdsAndRuleProfileUuids(ruleIds, chunk)); + } + public ActiveRuleDto insert(DbSession dbSession, ActiveRuleDto item) { - Preconditions.checkArgument(item.getProfileId() != null, QUALITY_PROFILE_IS_NOT_PERSISTED); - Preconditions.checkArgument(item.getRuleId() != null, RULE_IS_NOT_PERSISTED); - Preconditions.checkArgument(item.getId() == null, ACTIVE_RULE_IS_ALREADY_PERSISTED); + checkArgument(item.getProfileId() != null, QUALITY_PROFILE_IS_NOT_PERSISTED); + checkArgument(item.getRuleId() != null, RULE_IS_NOT_PERSISTED); + checkArgument(item.getId() == null, ACTIVE_RULE_IS_ALREADY_PERSISTED); mapper(dbSession).insert(item); return item; } public ActiveRuleDto update(DbSession dbSession, ActiveRuleDto item) { - Preconditions.checkArgument(item.getProfileId() != null, QUALITY_PROFILE_IS_NOT_PERSISTED); - Preconditions.checkArgument(item.getRuleId() != null, ActiveRuleDao.RULE_IS_NOT_PERSISTED); - Preconditions.checkArgument(item.getId() != null, ACTIVE_RULE_IS_NOT_PERSISTED); + checkArgument(item.getProfileId() != null, QUALITY_PROFILE_IS_NOT_PERSISTED); + checkArgument(item.getRuleId() != null, ActiveRuleDao.RULE_IS_NOT_PERSISTED); + checkArgument(item.getId() != null, ACTIVE_RULE_IS_NOT_PERSISTED); mapper(dbSession).update(item); return item; } @@ -128,8 +143,8 @@ public class ActiveRuleDao implements Dao { } public ActiveRuleParamDto insertParam(DbSession dbSession, ActiveRuleDto activeRule, ActiveRuleParamDto activeRuleParam) { - Preconditions.checkArgument(activeRule.getId() != null, ACTIVE_RULE_IS_NOT_PERSISTED); - Preconditions.checkArgument(activeRuleParam.getId() == null, ACTIVE_RULE_PARAM_IS_ALREADY_PERSISTED); + checkArgument(activeRule.getId() != null, ACTIVE_RULE_IS_NOT_PERSISTED); + checkArgument(activeRuleParam.getId() == null, ACTIVE_RULE_PARAM_IS_ALREADY_PERSISTED); Preconditions.checkNotNull(activeRuleParam.getRulesParameterId(), RULE_PARAM_IS_NOT_PERSISTED); activeRuleParam.setActiveRuleId(activeRule.getId()); @@ -199,4 +214,5 @@ public class ActiveRuleDao implements Dao { private static ActiveRuleMapper mapper(DbSession dbSession) { return dbSession.getMapper(ActiveRuleMapper.class); } + } 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 fed79f370c8..a9f2169bc1b 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 @@ -55,6 +55,10 @@ public interface ActiveRuleMapper { List selectByRuleProfileUuid(@Param("ruleProfileUuid") String uuid); + List selectByRuleIdsAndRuleProfileUuids( + @Param("ruleIds") Collection ruleIds, + @Param("ruleProfileUuids") Collection ruleProfileUuids); + void insertParameter(ActiveRuleParamDto dto); void updateParameter(ActiveRuleParamDto dto); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java index b6e78c118bd..8820d57cad0 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java @@ -233,8 +233,8 @@ public class QualityProfileDao implements Dao { DatabaseUtils.executeLargeUpdates(rulesProfileUuids, mapper::deleteRuleProfilesByUuids); } - public List selectChildrenOfBuiltInRulesProfile(DbSession dbSession, RulesProfileDto rulesProfile) { - return mapper(dbSession).selectChildrenOfBuiltInRulesProfile(rulesProfile.getKee()); + public List selectQProfilesByRuleProfile(DbSession dbSession, RulesProfileDto rulesProfile) { + return mapper(dbSession).selectQProfilesByRuleProfileUuid(rulesProfile.getKee()); } private static String sqlQueryString(@Nullable String query) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java index 5027a029d63..5dcb7fc1a61 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java @@ -125,5 +125,5 @@ public interface QualityProfileMapper { void renameRuleProfiles(@Param("newName") String newName, @Param("updatedAt") Date updatedAt, @Param("uuids") Collection uuids); - List selectChildrenOfBuiltInRulesProfile(@Param("rulesProfileUuid") String rulesProfileUuid); + List selectQProfilesByRuleProfileUuid(@Param("rulesProfileUuid") String rulesProfileUuid); } 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 2240344cda0..7c8d608806c 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 @@ -132,6 +132,16 @@ rp.kee = #{ruleProfileUuid, jdbcType=VARCHAR} + + + 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 68a02cc5620..d08fbd4542f 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 @@ -20,6 +20,7 @@ package org.sonar.db.qualityprofile; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.function.Consumer; @@ -40,7 +41,6 @@ import org.sonar.db.rule.RuleParamDto; 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; @@ -108,7 +108,7 @@ public class ActiveRuleDaoTest { } @Test - public void select_by_rule() { + public void selectByRuleId() { ActiveRuleDto activeRule1 = createFor(profile1, rule1).setSeverity(BLOCKER); ActiveRuleDto activeRule2 = createFor(profile2, rule1).setSeverity(BLOCKER); underTest.insert(dbSession, activeRule1); @@ -120,7 +120,7 @@ public class ActiveRuleDaoTest { } @Test - public void select_by_rule_ids() { + public void selectByRuleIds() { ActiveRuleDto activeRule1 = createFor(profile1, rule1).setSeverity(BLOCKER); ActiveRuleDto activeRule2 = createFor(profile1, rule2).setSeverity(BLOCKER); ActiveRuleDto activeRule3 = createFor(profile2, rule1).setSeverity(BLOCKER); @@ -129,7 +129,7 @@ public class ActiveRuleDaoTest { underTest.insert(dbSession, activeRule3); dbSession.commit(); - assertThat(underTest.selectByRuleIds(dbSession, organization, singletonList(rule1.getId()))) + assertThat(underTest.selectByRuleIds(dbSession, organization, asList(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()); @@ -160,7 +160,7 @@ public class ActiveRuleDaoTest { } @Test - public void selectByRuleProfileUuid() { + public void selectByRuleProfile() { ActiveRuleDto activeRule1 = createFor(profile1, rule1).setSeverity(BLOCKER); ActiveRuleDto activeRule2 = createFor(profile1, rule2).setSeverity(MAJOR); underTest.insert(dbSession, activeRule1); @@ -175,6 +175,42 @@ public class ActiveRuleDaoTest { assertThat(underTest.selectByProfile(dbSession, profile2)).isEmpty(); } + @Test + public void selectByRulesAndRuleProfileUuids() { + ActiveRuleDto rule1P1 = createFor(profile1, rule1).setSeverity(MAJOR); + ActiveRuleDto rule2P1 = createFor(profile1, rule2).setSeverity(MAJOR); + ActiveRuleDto rule1P2 = createFor(profile2, rule1).setSeverity(MAJOR); + underTest.insert(dbSession, rule1P1); + underTest.insert(dbSession, rule2P1); + underTest.insert(dbSession, rule1P2); + + // empty rules + Collection result = underTest.selectByRulesAndRuleProfileUuids(dbSession, emptyList(), asList(profile1.getRulesProfileUuid())); + assertThat(result).isEmpty(); + + // empty profiles + result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1), emptyList()); + assertThat(result).isEmpty(); + + // match + result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid())); + assertThat(result) + .extracting(ActiveRuleDto::getId) + .containsExactlyInAnyOrder(rule1P1.getId(), rule1P2.getId()); + + result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1, rule2), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid())); + assertThat(result) + .extracting(ActiveRuleDto::getId) + .containsExactlyInAnyOrder(rule1P1.getId(), rule1P2.getId(), rule2P1.getId()); + + // do not match + result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule3), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid())); + assertThat(result).isEmpty(); + + result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1), asList("unknown")); + assertThat(result).isEmpty(); + } + @Test public void insert() { ActiveRuleDto activeRule = createFor(profile1, rule1) @@ -527,7 +563,7 @@ public class ActiveRuleDaoTest { 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(profileWithoutActiveRule)).build())).isEmpty(); assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1, profile2, profileWithoutActiveRule)).build())).containsOnly( entry(profile1.getKee(), 2L), entry(profile2.getKee(), 1L)); @@ -547,9 +583,9 @@ public class ActiveRuleDaoTest { 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())) + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1)).setRuleStatus(READY).build())) .containsOnly(entry(profile1.getKee(), 2L)); - assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(singletonList(profile1)).setRuleStatus(REMOVED).build())) + assertThat(underTest.countActiveRulesByQuery(dbSession, builder.setProfiles(asList(profile1)).setRuleStatus(REMOVED).build())) .containsOnly(entry(profile1.getKee(), 1L)); } @@ -577,7 +613,7 @@ public class ActiveRuleDaoTest { assertThat(underTest.countActiveRulesByQuery(dbSession, ActiveRuleCountQuery.builder().setOrganization(organization).setProfiles(asList(profile1, profileOnAnotherOrganization)).build())) - .containsOnly(entry(profile1.getKee(), 1L)); + .containsOnly(entry(profile1.getKee(), 1L)); } @Test @@ -599,9 +635,9 @@ public class ActiveRuleDaoTest { .extracting(IndexedActiveRuleDto::getId, IndexedActiveRuleDto::getRepository, IndexedActiveRuleDto::getKey, IndexedActiveRuleDto::getRuleProfileUuid, IndexedActiveRuleDto::getSeverity, IndexedActiveRuleDto::getInheritance) .containsExactlyInAnyOrder( - tuple((long)ar1.getId(), ar1.getRuleKey().repository(), ar1.getRuleKey().rule(), profile1.getRulesProfileUuid(), ar1.getSeverity(), ar1.getInheritance()), - tuple((long)ar2.getId(), ar2.getRuleKey().repository(), ar2.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar2.getSeverity(), ar2.getInheritance()), - tuple((long)ar3.getId(), ar3.getRuleKey().repository(), ar3.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar3.getSeverity(), ar3.getInheritance())); + tuple((long) ar1.getId(), ar1.getRuleKey().repository(), ar1.getRuleKey().rule(), profile1.getRulesProfileUuid(), ar1.getSeverity(), ar1.getInheritance()), + tuple((long) ar2.getId(), ar2.getRuleKey().repository(), ar2.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar2.getSeverity(), ar2.getInheritance()), + tuple((long) ar3.getId(), ar3.getRuleKey().repository(), ar3.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar3.getSeverity(), ar3.getInheritance())); } @Test @@ -611,13 +647,13 @@ public class ActiveRuleDaoTest { ActiveRuleDto ar3 = db.qualityProfiles().activateRule(profile2, rule2); Accumulator accumulator = new Accumulator(); - underTest.scrollByIdsForIndexing(dbSession, asList((long)ar1.getId(), (long)ar2.getId()), accumulator); + underTest.scrollByIdsForIndexing(dbSession, asList((long) ar1.getId(), (long) ar2.getId()), accumulator); assertThat(accumulator.list) .extracting(IndexedActiveRuleDto::getId, IndexedActiveRuleDto::getRepository, IndexedActiveRuleDto::getKey, IndexedActiveRuleDto::getRuleProfileUuid, IndexedActiveRuleDto::getSeverity) .containsExactlyInAnyOrder( - tuple((long)ar1.getId(), ar1.getRuleKey().repository(), ar1.getRuleKey().rule(), profile1.getRulesProfileUuid(), ar1.getSeverity()), - tuple((long)ar2.getId(), ar2.getRuleKey().repository(), ar2.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar2.getSeverity())); + tuple((long) ar1.getId(), ar1.getRuleKey().repository(), ar1.getRuleKey().rule(), profile1.getRulesProfileUuid(), ar1.getSeverity()), + tuple((long) ar2.getId(), ar2.getRuleKey().repository(), ar2.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar2.getSeverity())); } @Test @@ -632,8 +668,8 @@ public class ActiveRuleDaoTest { .extracting(IndexedActiveRuleDto::getId, IndexedActiveRuleDto::getRepository, IndexedActiveRuleDto::getKey, IndexedActiveRuleDto::getRuleProfileUuid, IndexedActiveRuleDto::getSeverity) .containsExactlyInAnyOrder( - tuple((long)ar2.getId(), ar2.getRuleKey().repository(), ar2.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar2.getSeverity()), - tuple((long)ar3.getId(), ar3.getRuleKey().repository(), ar3.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar3.getSeverity())); + tuple((long) ar2.getId(), ar2.getRuleKey().repository(), ar2.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar2.getSeverity()), + tuple((long) ar3.getId(), ar3.getRuleKey().repository(), ar3.getRuleKey().rule(), profile2.getRulesProfileUuid(), ar3.getSeverity())); } private static class Accumulator implements Consumer { diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java index 3f728a0d2a3..5e00dba9177 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java @@ -49,9 +49,7 @@ 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.core.util.stream.MoreCollectors.toList; import static org.sonar.db.qualityprofile.QualityProfileTesting.newQualityProfileDto; -import static org.sonar.db.qualityprofile.RulesProfileDto.from; public class QualityProfileDaoTest { @@ -706,6 +704,36 @@ public class QualityProfileDaoTest { assertThat(underTest.selectOrFailByUuid(dbSession, profile.getKee()).getName()).isEqualTo("foo"); } + @Test + public void selectQProfilesByRuleProfileUuid() { + OrganizationDto org1 = db.organizations().insert(); + OrganizationDto org2 = db.organizations().insert(); + + RulesProfileDto ruleProfile1 = QualityProfileTesting.newRuleProfileDto(); + OrgQProfileDto profile1InOrg1 = new OrgQProfileDto().setOrganizationUuid(org1.getUuid()).setRulesProfileUuid(ruleProfile1.getKee()).setUuid(Uuids.create()); + OrgQProfileDto profile1InOrg2 = new OrgQProfileDto().setOrganizationUuid(org2.getUuid()).setRulesProfileUuid(ruleProfile1.getKee()).setUuid(Uuids.create()); + RulesProfileDto ruleProfile2 = QualityProfileTesting.newRuleProfileDto(); + OrgQProfileDto profile2InOrg1 = new OrgQProfileDto().setOrganizationUuid(org1.getUuid()).setRulesProfileUuid(ruleProfile2.getKee()).setUuid(Uuids.create()); + db.getDbClient().qualityProfileDao().insert(db.getSession(), ruleProfile1); + db.getDbClient().qualityProfileDao().insert(db.getSession(), profile1InOrg1); + db.getDbClient().qualityProfileDao().insert(db.getSession(), profile1InOrg2); + db.getDbClient().qualityProfileDao().insert(db.getSession(), ruleProfile2); + db.getDbClient().qualityProfileDao().insert(db.getSession(), profile2InOrg1); + + List result = db.getDbClient().qualityProfileDao().selectQProfilesByRuleProfile(db.getSession(), ruleProfile1); + assertThat(result).extracting(QProfileDto::getKee).containsExactlyInAnyOrder(profile1InOrg1.getUuid(), profile1InOrg2.getUuid()); + + result = db.getDbClient().qualityProfileDao().selectQProfilesByRuleProfile(db.getSession(), ruleProfile2); + assertThat(result).extracting(QProfileDto::getKee).containsExactlyInAnyOrder(profile2InOrg1.getUuid()); + } + + @Test + public void selectQProfilesByRuleProfileUuid_returns_empty_list_if_rule_profile_does_not_exist() { + List result = db.getDbClient().qualityProfileDao().selectQProfilesByRuleProfile(db.getSession(), new RulesProfileDto().setKee("unknown")); + + assertThat(result).isEmpty(); + } + private List createSharedData() { QProfileDto dto1 = new QProfileDto() .setKee("java_sonar_way") @@ -739,36 +767,4 @@ public class QualityProfileDaoTest { return Arrays.asList(dto1, dto2); } - - @Test - public void selectChildrenOfBuiltInRulesProfile_must_return_only_inherited_profiles() { - OrganizationDto org1 = db.organizations().insert(); - OrganizationDto org2 = db.organizations().insert(); - OrganizationDto org3 = db.organizations().insert(); - - QProfileDto builtInProfile = db.qualityProfiles().insert(org1, p -> p.setIsBuiltIn(true).setLanguage("java").setName("foo")); - QProfileDto javaProfileOrg2 = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo")); - QProfileDto inheritedJavaProfileOrg2 = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo").setParentKee(builtInProfile.getKee())); - QProfileDto differentLanguage = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("cobol").setName("foo")); - QProfileDto differentName = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("java").setName("bar")); - QProfileDto javaProfileOrg3 = db.qualityProfiles().insert(org3, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo")); - QProfileDto inheritedJavaProfileOrg3 = db.qualityProfiles().insert(org3, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo").setParentKee(builtInProfile.getKee())); - - List children = db.getDbClient().qualityProfileDao().selectChildrenOfBuiltInRulesProfile(db.getSession(), from(builtInProfile)); - - assertThat(children.stream().map(qp -> qp.getId()).collect(toList())).containsExactlyInAnyOrder( - inheritedJavaProfileOrg2.getId(), inheritedJavaProfileOrg3.getId()); - } - - @Test - public void selectChildrenOfBuiltInRulesProfile_must_return_empty_list_if_not_built_in() { - OrganizationDto org = db.organizations().insert(); - - QProfileDto notBuiltInProfile = db.qualityProfiles().insert(org, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo")); - QProfileDto inheritedProfile = db.qualityProfiles().insert(org, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo").setParentKee(notBuiltInProfile.getKee())); - - List children = db.getDbClient().qualityProfileDao().selectChildrenOfBuiltInRulesProfile(db.getSession(), from(notBuiltInProfile)); - - assertThat(children).isEmpty(); - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java index 89cf7930d08..7de860aff00 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java @@ -167,8 +167,9 @@ import org.sonar.server.qualityprofile.QProfileCopier; import org.sonar.server.qualityprofile.QProfileExporters; import org.sonar.server.qualityprofile.QProfileFactoryImpl; import org.sonar.server.qualityprofile.QProfileResetImpl; +import org.sonar.server.qualityprofile.QProfileRulesImpl; +import org.sonar.server.qualityprofile.QProfileTreeImpl; import org.sonar.server.qualityprofile.RuleActivator; -import org.sonar.server.qualityprofile.RuleActivatorContextFactory; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.qualityprofile.ws.ProfilesWs; import org.sonar.server.qualityprofile.ws.QProfilesWsModule; @@ -306,9 +307,10 @@ public class PlatformLevel4 extends PlatformLevel { AnnotationProfileParser.class, QProfileComparison.class, ProfilesWs.class, + QProfileTreeImpl.class, + QProfileRulesImpl.class, RuleActivator.class, QProfileExporters.class, - RuleActivatorContextFactory.class, QProfileFactoryImpl.class, QProfileCopier.class, QProfileBackuperImpl.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImpl.java index 29ee43ac1e0..04b83baa134 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImpl.java @@ -20,6 +20,8 @@ package org.sonar.server.qualityprofile; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -45,22 +47,31 @@ public class BuiltInQProfileUpdateImpl implements BuiltInQProfileUpdate { this.activeRuleIndexer = activeRuleIndexer; } - public List update(DbSession dbSession, BuiltInQProfile builtIn, RulesProfileDto ruleProfile) { + public List update(DbSession dbSession, BuiltInQProfile builtIn, RulesProfileDto rulesProfile) { // Keep reference to all the activated rules before update - Set toBeDeactivated = dbClient.activeRuleDao().selectByRuleProfile(dbSession, ruleProfile) + Set deactivatedKeys = dbClient.activeRuleDao().selectByRuleProfile(dbSession, rulesProfile) .stream() .map(ActiveRuleDto::getRuleKey) .collect(MoreCollectors.toHashSet()); - List changes = new ArrayList<>(); - builtIn.getActiveRules().forEach(ar -> { + Collection activations = new ArrayList<>(); + Collection ruleKeys = new HashSet<>(deactivatedKeys); + for (BuiltInActiveRule ar : builtIn.getActiveRules()) { RuleActivation activation = convert(ar); - toBeDeactivated.remove(activation.getRuleKey()); - changes.addAll(ruleActivator.activateOnBuiltInRulesProfile(dbSession, activation, ruleProfile)); - }); + activations.add(activation); + ruleKeys.add(activation.getRuleKey()); + deactivatedKeys.remove(activation.getRuleKey()); + } + + RuleActivationContext context = ruleActivator.createContextForBuiltInProfile(dbSession, rulesProfile, ruleKeys); + + List changes = new ArrayList<>(); + for (RuleActivation activation : activations) { + changes.addAll(ruleActivator.activate(dbSession, activation, context)); + } // these rules are not part of the built-in profile anymore - toBeDeactivated.forEach(ruleKey -> changes.addAll(ruleActivator.deactivateOnBuiltInRulesProfile(dbSession, ruleProfile, ruleKey, false))); + deactivatedKeys.forEach(ruleKey -> changes.addAll(ruleActivator.deactivate(dbSession, context, ruleKey, false))); activeRuleIndexer.commitAndIndex(dbSession, changes); return changes; diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java index a3e02be69e0..3cbaa9f121c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java @@ -36,6 +36,7 @@ import org.sonar.api.profiles.ProfileExporter; import org.sonar.api.profiles.ProfileImporter; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.ActiveRule; import org.sonar.api.rules.ActiveRuleParam; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; @@ -59,14 +60,14 @@ public class QProfileExporters { private final DbClient dbClient; private final RuleFinder ruleFinder; - private final RuleActivator ruleActivator; + private final QProfileRules qProfileRules; private final ProfileExporter[] exporters; private final ProfileImporter[] importers; - public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, RuleActivator ruleActivator, ProfileExporter[] exporters, ProfileImporter[] importers) { + public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, QProfileRules qProfileRules, ProfileExporter[] exporters, ProfileImporter[] importers) { this.dbClient = dbClient; this.ruleFinder = ruleFinder; - this.ruleActivator = ruleActivator; + this.qProfileRules = qProfileRules; this.exporters = exporters; this.importers = importers; } @@ -74,22 +75,22 @@ public class QProfileExporters { /** * Used by Pico if no {@link ProfileImporter} is found */ - public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, RuleActivator ruleActivator, ProfileExporter[] exporters) { - this(dbClient, ruleFinder, ruleActivator, exporters, new ProfileImporter[0]); + public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, QProfileRules qProfileRules, ProfileExporter[] exporters) { + this(dbClient, ruleFinder, qProfileRules, exporters, new ProfileImporter[0]); } /** * Used by Pico if no {@link ProfileExporter} is found */ - public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, RuleActivator ruleActivator, ProfileImporter[] importers) { - this(dbClient, ruleFinder, ruleActivator, new ProfileExporter[0], importers); + public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, QProfileRules qProfileRules, ProfileImporter[] importers) { + this(dbClient, ruleFinder, qProfileRules, new ProfileExporter[0], importers); } /** * Used by Pico if no {@link ProfileImporter} nor {@link ProfileExporter} is found */ - public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, RuleActivator ruleActivator) { - this(dbClient, ruleFinder, ruleActivator, new ProfileExporter[0], new ProfileImporter[0]); + public QProfileExporters(DbClient dbClient, RuleFinder ruleFinder, QProfileRules qProfileRules) { + this(dbClient, ruleFinder, qProfileRules, new ProfileExporter[0], new ProfileImporter[0]); } public List exportersForLanguage(String language) { @@ -139,27 +140,27 @@ public class QProfileExporters { throw new NotFoundException("Unknown quality profile exporter: " + exporterKey); } - public QProfileResult importXml(QProfileDto profileDto, String importerKey, InputStream xml, DbSession dbSession) { - return importXml(profileDto, importerKey, new InputStreamReader(xml, StandardCharsets.UTF_8), dbSession); + public QProfileResult importXml(QProfileDto profile, String importerKey, InputStream xml, DbSession dbSession) { + return importXml(profile, importerKey, new InputStreamReader(xml, StandardCharsets.UTF_8), dbSession); } - private QProfileResult importXml(QProfileDto profileDto, String importerKey, Reader xml, DbSession dbSession) { + private QProfileResult importXml(QProfileDto profile, String importerKey, Reader xml, DbSession dbSession) { QProfileResult result = new QProfileResult(); ValidationMessages messages = ValidationMessages.create(); ProfileImporter importer = getProfileImporter(importerKey); - RulesProfile rulesProfile = importer.importProfile(xml, messages); - List changes = importProfile(profileDto, rulesProfile, dbSession); + RulesProfile definition = importer.importProfile(xml, messages); + List changes = importProfile(profile, definition, dbSession); result.addChanges(changes); processValidationMessages(messages, result); return result; } - private List importProfile(QProfileDto profileDto, RulesProfile rulesProfile, DbSession dbSession) { - List changes = new ArrayList<>(); - for (org.sonar.api.rules.ActiveRule activeRule : rulesProfile.getActiveRules()) { - changes.addAll(ruleActivator.activate(dbSession, toRuleActivation(activeRule), profileDto)); - } - return changes; + private List importProfile(QProfileDto profile, RulesProfile definition, DbSession dbSession) { + List activeRules = definition.getActiveRules(); + List activations = activeRules.stream() + .map(QProfileExporters::toRuleActivation) + .collect(MoreCollectors.toArrayList(activeRules.size())); + return qProfileRules.activateAndCommit(dbSession, profile, activations); } private ProfileImporter getProfileImporter(String importerKey) { @@ -177,7 +178,7 @@ public class QProfileExporters { result.addInfos(messages.getInfos()); } - private static RuleActivation toRuleActivation(org.sonar.api.rules.ActiveRule activeRule) { + private static RuleActivation toRuleActivation(ActiveRule activeRule) { RuleKey ruleKey = activeRule.getRule().ruleKey(); String severity = activeRule.getSeverity().name(); Map params = activeRule.getActiveRuleParams().stream() diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileResetImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileResetImpl.java index 372130a64ff..f7f6459fd5c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileResetImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileResetImpl.java @@ -53,20 +53,24 @@ public class QProfileResetImpl implements QProfileReset { public BulkChangeResult reset(DbSession dbSession, QProfileDto profile, Collection activations) { requireNonNull(profile.getId(), "Quality profile must be persisted"); checkArgument(!profile.isBuiltIn(), "Operation forbidden for built-in Quality Profile '%s'", profile.getKee()); + BulkChangeResult result = new BulkChangeResult(); - Set ruleToBeDeactivated = new HashSet<>(); + Set rulesToBeDeactivated = new HashSet<>(); // Keep reference to all the activated rules before backup restore for (ActiveRuleDto activeRuleDto : db.activeRuleDao().selectByProfile(dbSession, profile)) { if (activeRuleDto.getInheritance() == null) { // inherited rules can't be deactivated - ruleToBeDeactivated.add(activeRuleDto.getRuleKey()); + rulesToBeDeactivated.add(activeRuleDto.getRuleKey()); } } + Set ruleKeys = new HashSet<>(rulesToBeDeactivated); + activations.forEach(a -> ruleKeys.add(a.getRuleKey())); + RuleActivationContext context = activator.createContextForUserProfile(dbSession, profile, ruleKeys); for (RuleActivation activation : activations) { try { - List changes = activator.activate(dbSession, activation, profile); - ruleToBeDeactivated.remove(activation.getRuleKey()); + List changes = activator.activate(dbSession, activation, context); + rulesToBeDeactivated.remove(activation.getRuleKey()); result.incrementSucceeded(); result.addChanges(changes); } catch (BadRequestException e) { @@ -75,11 +79,10 @@ public class QProfileResetImpl implements QProfileReset { } } - List changes = new ArrayList<>(); - changes.addAll(result.getChanges()); - for (RuleKey ruleKey : ruleToBeDeactivated) { + List changes = new ArrayList<>(result.getChanges()); + for (RuleKey ruleKey : rulesToBeDeactivated) { try { - changes.addAll(activator.deactivate(dbSession, profile, ruleKey)); + changes.addAll(activator.deactivate(dbSession, context, ruleKey, false)); } catch (BadRequestException e) { // ignore, probably a rule inherited from parent that can't be deactivated } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRules.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRules.java new file mode 100644 index 00000000000..57d017cbe72 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRules.java @@ -0,0 +1,66 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 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.server.qualityprofile; + +import java.util.Collection; +import java.util.List; +import javax.annotation.Nullable; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.server.ServerSide; +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.server.rule.index.RuleQuery; + +/** + * Operations related to activation and deactivation of rules on user profiles. + * Use {@link BuiltInQProfileUpdate} for built-in profiles. + */ +@ServerSide +public interface QProfileRules { + + /** + * Activate multiple rules at once on a Quality profile. + * Db session is committed and Elasticsearch indices are updated. + * If an activation fails to be executed, then all others are + * canceled, db session is not committed and an exception is + * thrown. + */ + List activateAndCommit(DbSession dbSession, QProfileDto profile, Collection activations); + + /** + * Same as {@link #activateAndCommit(DbSession, QProfileDto, Collection)} except + * that: + * - rules are loaded from search engine + * - rules are activated with default parameters + * - an activation failure does not break others. No exception is thrown. + */ + BulkChangeResult bulkActivateAndCommit(DbSession dbSession, QProfileDto profile, RuleQuery ruleQuery, @Nullable String severity); + + List deactivateAndCommit(DbSession dbSession, QProfileDto profile, Collection ruleKeys); + + BulkChangeResult bulkDeactivateAndCommit(DbSession dbSession, QProfileDto profile, RuleQuery ruleQuery); + + /** + * Delete a rule from all Quality profiles. Db session is not committed. As a + * consequence Elasticsearch indices are NOT updated. + */ + List deleteRule(DbSession dbSession, RuleDefinitionDto rule); +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRulesImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRulesImpl.java new file mode 100644 index 00000000000..262a65de13f --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRulesImpl.java @@ -0,0 +1,139 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 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.server.qualityprofile; + +import com.google.common.collect.Sets; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Set; +import java.util.function.BiFunction; +import javax.annotation.Nullable; +import org.sonar.api.rule.RuleKey; +import org.sonar.core.util.stream.MoreCollectors; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; +import org.sonar.server.rule.index.RuleIndex; +import org.sonar.server.rule.index.RuleQuery; + +import static com.google.common.base.Preconditions.checkArgument; + +public class QProfileRulesImpl implements QProfileRules { + + private final DbClient db; + private final RuleActivator ruleActivator; + private final RuleIndex ruleIndex; + private final ActiveRuleIndexer activeRuleIndexer; + + public QProfileRulesImpl(DbClient db, RuleActivator ruleActivator, RuleIndex ruleIndex, ActiveRuleIndexer activeRuleIndexer) { + this.db = db; + this.ruleActivator = ruleActivator; + this.ruleIndex = ruleIndex; + this.activeRuleIndexer = activeRuleIndexer; + } + + @Override + public List activateAndCommit(DbSession dbSession, QProfileDto profile, Collection activations) { + verifyNotBuiltIn(profile); + + Set ruleKeys = activations.stream().map(RuleActivation::getRuleKey).collect(MoreCollectors.toHashSet(activations.size())); + RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, ruleKeys); + + List changes = new ArrayList<>(); + for (RuleActivation activation : activations) { + changes.addAll(ruleActivator.activate(dbSession, activation, context)); + } + activeRuleIndexer.commitAndIndex(dbSession, changes); + return changes; + } + + @Override + public BulkChangeResult bulkActivateAndCommit(DbSession dbSession, QProfileDto profile, RuleQuery ruleQuery, @Nullable String severity) { + verifyNotBuiltIn(profile); + return doBulk(dbSession, profile, ruleQuery, (context, ruleKey) -> { + RuleActivation activation = RuleActivation.create(ruleKey, severity, null); + return ruleActivator.activate(dbSession, activation, context); + }); + } + + @Override + public List deactivateAndCommit(DbSession dbSession, QProfileDto profile, Collection ruleKeys) { + verifyNotBuiltIn(profile); + RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, ruleKeys); + + List changes = new ArrayList<>(); + for (RuleKey ruleKey : ruleKeys) { + changes.addAll(ruleActivator.deactivate(dbSession, context, ruleKey, false)); + } + activeRuleIndexer.commitAndIndex(dbSession, changes); + return changes; + } + + @Override + public BulkChangeResult bulkDeactivateAndCommit(DbSession dbSession, QProfileDto profile, RuleQuery ruleQuery) { + verifyNotBuiltIn(profile); + return doBulk(dbSession, profile, ruleQuery, (context, ruleKey) -> ruleActivator.deactivate(dbSession, context, ruleKey, false)); + } + + @Override + public List deleteRule(DbSession dbSession, RuleDefinitionDto rule) { + List changes = new ArrayList<>(); + List activeRuleIds = new ArrayList<>(); + db.activeRuleDao().selectByRuleIdOfAllOrganizations(dbSession, rule.getId()).forEach(ar -> { + activeRuleIds.add(ar.getId()); + changes.add(new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, ar)); + }); + + db.activeRuleDao().deleteByIds(dbSession, activeRuleIds); + db.activeRuleDao().deleteParamsByActiveRuleIds(dbSession, activeRuleIds); + + return changes; + } + + private static void verifyNotBuiltIn(QProfileDto profile) { + checkArgument(!profile.isBuiltIn(), "The built-in profile %s is read-only and can't be updated", profile.getName()); + } + + private BulkChangeResult doBulk(DbSession dbSession, QProfileDto profile, RuleQuery ruleQuery, BiFunction> fn) { + BulkChangeResult result = new BulkChangeResult(); + Collection ruleKeys = Sets.newHashSet(ruleIndex.searchAll(ruleQuery)); + RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, ruleKeys); + + for (RuleKey ruleKey : ruleKeys) { + try { + List changes = fn.apply(context, ruleKey); + result.addChanges(changes); + if (!changes.isEmpty()) { + result.incrementSucceeded(); + } + } catch (BadRequestException e) { + // other exceptions stop the bulk activation + result.incrementFailed(); + result.getErrors().addAll(e.errors()); + } + } + activeRuleIndexer.commitAndIndex(dbSession, result.getChanges()); + return result; + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTree.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTree.java new file mode 100644 index 00000000000..428a7e050bd --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTree.java @@ -0,0 +1,37 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 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.server.qualityprofile; + +import java.util.List; +import org.sonar.api.server.ServerSide; +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.QProfileDto; + +/** + * Operations related to hierarchy of profiles + */ +@ServerSide +public interface QProfileTree { + + List removeParentAndCommit(DbSession dbSession, QProfileDto profile); + + List setParentAndCommit(DbSession dbSession, QProfileDto profile, QProfileDto parentProfile); + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java new file mode 100644 index 00000000000..142833e0033 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java @@ -0,0 +1,141 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 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.server.qualityprofile; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import javax.annotation.Nullable; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.utils.System2; +import org.sonar.core.util.stream.MoreCollectors; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.ActiveRuleDto; +import org.sonar.db.qualityprofile.OrgActiveRuleDto; +import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; + +import static org.sonar.server.ws.WsUtils.checkRequest; + +public class QProfileTreeImpl implements QProfileTree { + + private final DbClient db; + private final RuleActivator ruleActivator; + private final System2 system2; + private final ActiveRuleIndexer activeRuleIndexer; + + public QProfileTreeImpl(DbClient db, RuleActivator ruleActivator, System2 system2, ActiveRuleIndexer activeRuleIndexer) { + this.db = db; + this.ruleActivator = ruleActivator; + this.system2 = system2; + this.activeRuleIndexer = activeRuleIndexer; + } + + @Override + public List removeParentAndCommit(DbSession dbSession, QProfileDto profile) { + List changes = removeParent(dbSession, profile); + activeRuleIndexer.commitAndIndex(dbSession, changes); + return changes; + } + + @Override + public List setParentAndCommit(DbSession dbSession, QProfileDto profile, QProfileDto parentProfile) { + List changes = setParent(dbSession, profile, parentProfile); + activeRuleIndexer.commitAndIndex(dbSession, changes); + return changes; + } + + private List setParent(DbSession dbSession, QProfileDto profile, QProfileDto parent) { + checkRequest(parent.getLanguage().equals(profile.getLanguage()), "Cannot set the profile '%s' as the parent of profile '%s' since their languages differ ('%s' != '%s')", + parent.getKee(), profile.getKee(), parent.getLanguage(), profile.getLanguage()); + + List changes = new ArrayList<>(); + if (parent.getKee().equals(profile.getParentKee())) { + return changes; + } + + checkRequest(!isDescendant(dbSession, profile, parent), "Descendant profile '%s' can not be selected as parent of '%s'", parent.getKee(), profile.getKee()); + changes.addAll(removeParent(dbSession, profile)); + + // set new parent + profile.setParentKee(parent.getKee()); + db.qualityProfileDao().update(dbSession, profile); + + List parentActiveRules = db.activeRuleDao().selectByProfile(dbSession, parent); + Collection ruleKeys = parentActiveRules.stream().map(ActiveRuleDto::getRuleKey).collect(MoreCollectors.toArrayList()); + RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, ruleKeys); + + for (ActiveRuleDto parentActiveRule : parentActiveRules) { + try { + RuleActivation activation = RuleActivation.create(parentActiveRule.getRuleKey(), null, null); + changes.addAll(ruleActivator.activate(dbSession, activation, context)); + } catch (BadRequestException e) { + // for example because rule status is REMOVED + // TODO return errors + } + } + return changes; + } + + private List removeParent(DbSession dbSession, QProfileDto profile) { + List changes = new ArrayList<>(); + if (profile.getParentKee() == null) { + return changes; + } + + profile.setParentKee(null); + db.qualityProfileDao().update(dbSession, profile); + + List activeRules = db.activeRuleDao().selectByProfile(dbSession, profile); + Collection ruleKeys = activeRules.stream().map(ActiveRuleDto::getRuleKey).collect(MoreCollectors.toArrayList()); + RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, ruleKeys); + + for (OrgActiveRuleDto activeRule : activeRules) { + if (ActiveRuleDto.INHERITED.equals(activeRule.getInheritance())) { + changes.addAll(ruleActivator.deactivate(dbSession, context, activeRule.getRuleKey(), true)); + + } else if (ActiveRuleDto.OVERRIDES.equals(activeRule.getInheritance())) { + activeRule.setInheritance(null); + activeRule.setUpdatedAt(system2.now()); + db.activeRuleDao().update(dbSession, activeRule); + changes.add(new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activeRule).setInheritance(null)); + } + } + return changes; + } + + private boolean isDescendant(DbSession dbSession, QProfileDto childProfile, @Nullable QProfileDto parentProfile) { + QProfileDto currentParent = parentProfile; + while (currentParent != null) { + if (childProfile.getName().equals(currentParent.getName())) { + return true; + } + String parentKey = currentParent.getParentKee(); + if (parentKey != null) { + currentParent = db.qualityProfileDao().selectByUuid(dbSession, parentKey); + } else { + currentParent = null; + } + } + return false; + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java index 2017931e574..714890f71fc 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java @@ -80,7 +80,7 @@ public class RegisterQualityProfiles { builtInQProfiles.forEach(builtIn -> { RulesProfileDto ruleProfile = persistedRuleProfiles.get(builtIn.getQProfileName()); if (ruleProfile == null) { - register(dbSession, batchDbSession, builtIn); + create(dbSession, batchDbSession, builtIn); } else { List changes = update(dbSession, builtIn, ruleProfile); changedProfiles.putAll(builtIn.getQProfileName(), changes.stream() @@ -104,7 +104,7 @@ public class RegisterQualityProfiles { .collect(MoreCollectors.uniqueIndex(rp -> new QProfileName(rp.getLanguage(), rp.getName()))); } - private void register(DbSession dbSession, DbSession batchDbSession, BuiltInQProfile builtIn) { + private void create(DbSession dbSession, DbSession batchDbSession, BuiltInQProfile builtIn) { LOGGER.info("Register profile {}", builtIn.getQProfileName()); renameOutdatedProfiles(dbSession, builtIn); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java new file mode 100644 index 00000000000..100628ea2fe --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java @@ -0,0 +1,335 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 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.server.qualityprofile; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.Maps; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import org.sonar.api.rule.RuleKey; +import org.sonar.db.qualityprofile.ActiveRuleDto; +import org.sonar.db.qualityprofile.ActiveRuleKey; +import org.sonar.db.qualityprofile.ActiveRuleParamDto; +import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.qualityprofile.RulesProfileDto; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleParamDto; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static java.util.Objects.requireNonNull; +import static org.sonar.core.util.stream.MoreCollectors.index; +import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; +import static org.sonar.server.ws.WsUtils.checkRequest; + +/** + * Cache of the data required to activate/deactivate + * multiple rules on a Quality profile, including + * the rule definitions, the rule parameters, the tree + * of profiles hierarchy and its related active rules. + */ +class RuleActivationContext { + + private final long date; + + // the profiles + private RulesProfileDto baseRulesProfile; + @Nullable + private QProfileDto baseProfile; + private final Map profilesByUuid = new HashMap<>(); + private final ListMultimap profilesByParentUuid = ArrayListMultimap.create(); + private final List builtInAliases = new ArrayList<>(); + + // the rules + private final Map rulesByKey; + private final Map activeRulesByKey; + + // cursor, moved in the tree of profiles + private boolean cascading = false; + private RulesProfileDto currentRulesProfile; + @Nullable + private QProfileDto currentProfile; + @Nullable + private RuleWrapper currentRule; + @Nullable + private ActiveRuleWrapper currentActiveRule; + @Nullable + private ActiveRuleWrapper currentParentActiveRule; + + private RuleActivationContext(Builder builder) { + this.date = builder.date; + + // rules + this.rulesByKey = Maps.newHashMapWithExpectedSize(builder.rules.size()); + ListMultimap paramsByRuleId = builder.ruleParams.stream().collect(index(RuleParamDto::getRuleId)); + for (RuleDefinitionDto rule : builder.rules) { + RuleWrapper wrapper = new RuleWrapper(rule, paramsByRuleId.get(rule.getId())); + rulesByKey.put(rule.getKey(), wrapper); + } + + // profiles + this.baseProfile = builder.baseProfile; + this.baseRulesProfile = builder.baseRulesProfile; + for (QProfileDto profile : builder.profiles) { + profilesByUuid.put(profile.getKee(), profile); + if (profile.isBuiltIn()) { + builtInAliases.add(profile); + } else if (profile.getParentKee() != null) { + profilesByParentUuid.put(profile.getParentKee(), profile); + } + } + + // active rules + this.activeRulesByKey = Maps.newHashMapWithExpectedSize(builder.activeRules.size()); + ListMultimap paramsByActiveRuleId = builder.activeRuleParams.stream().collect(index(ActiveRuleParamDto::getActiveRuleId)); + for (ActiveRuleDto activeRule : builder.activeRules) { + ActiveRuleWrapper wrapper = new ActiveRuleWrapper(activeRule, paramsByActiveRuleId.get(activeRule.getId())); + this.activeRulesByKey.put(activeRule.getKey(), wrapper); + } + } + + long getDate() { + return date; + } + + RuleWrapper getRule() { + return currentRule; + } + + @CheckForNull + String getRequestedParamValue(RuleActivation request, String key) { + if (currentRule.rule.isCustomRule()) { + return null; + } + return request.getParameter(key); + } + + boolean hasRequestedParamValue(RuleActivation request, String key) { + return request.hasParameter(key); + } + + RulesProfileDto getRulesProfile() { + return currentRulesProfile; + } + + @CheckForNull + ActiveRuleWrapper getActiveRule() { + return currentActiveRule; + } + + @CheckForNull + ActiveRuleWrapper getParentActiveRule() { + return currentParentActiveRule; + } + + boolean isCascading() { + return cascading; + } + + @CheckForNull + QProfileDto getProfile() { + return currentProfile; + } + + Collection getChildProfiles() { + if (currentProfile != null) { + return profilesByParentUuid.get(currentProfile.getKee()); + } + // on built-in profile + checkState(currentRulesProfile.isBuiltIn()); + return builtInAliases.stream() + .flatMap(a -> profilesByParentUuid.get(a.getKee()).stream()) + .collect(Collectors.toList()); + } + + /** + * Resets cursor to base profile and selects the rule with specified key. + */ + void reset(RuleKey ruleKey) { + this.cascading = false; + doSwitch(this.baseProfile, this.baseRulesProfile, ruleKey); + } + + /** + * Moves cursor to a child profile + */ + void switchToChild(QProfileDto to) { + checkState(!to.isBuiltIn()); + requireNonNull(this.currentRule, "can not switch profile if rule is not set"); + RuleKey ruleKey = this.currentRule.get().getKey(); + + QProfileDto qp = requireNonNull(this.profilesByUuid.get(to.getKee()), () -> "No profile with uuid " + to.getKee()); + RulesProfileDto rulesProfile = RulesProfileDto.from(qp); + + this.cascading = true; + doSwitch(qp, rulesProfile, ruleKey); + } + + private void doSwitch(@Nullable QProfileDto qp, RulesProfileDto rulesProfile, RuleKey ruleKey) { + this.currentRule = rulesByKey.get(ruleKey); + checkRequest(this.currentRule != null, "Rule not found: %s", ruleKey); + checkRequest(rulesProfile.getLanguage().equals(currentRule.get().getLanguage()), + "%s rule %s cannot be activated on %s profile %s", currentRule.get().getLanguage(), currentRule.get().getKey(), rulesProfile.getLanguage(), rulesProfile.getName()); + this.currentRulesProfile = rulesProfile; + this.currentProfile = qp; + this.currentActiveRule = this.activeRulesByKey.get(ActiveRuleKey.of(rulesProfile, ruleKey)); + this.currentParentActiveRule = null; + if (this.currentProfile != null) { + String parentUuid = this.currentProfile.getParentKee(); + if (parentUuid != null) { + QProfileDto parent = requireNonNull(this.profilesByUuid.get(parentUuid), () -> "No profile with uuid " + parentUuid); + this.currentParentActiveRule = this.activeRulesByKey.get(ActiveRuleKey.of(parent, ruleKey)); + } + } + } + + static final class Builder { + private long date = System.currentTimeMillis(); + private RulesProfileDto baseRulesProfile; + private QProfileDto baseProfile; + private Collection rules; + private Collection ruleParams; + private Collection profiles; + private Collection activeRules; + private Collection activeRuleParams; + + Builder setDate(long l) { + this.date = l; + return this; + } + + Builder setBaseProfile(RulesProfileDto p) { + this.baseRulesProfile = p; + this.baseProfile = null; + return this; + } + + Builder setBaseProfile(QProfileDto p) { + this.baseRulesProfile = RulesProfileDto.from(p); + this.baseProfile = p; + return this; + } + + Builder setRules(Collection rules) { + this.rules = rules; + return this; + } + + Builder setRuleParams(Collection ruleParams) { + this.ruleParams = ruleParams; + return this; + } + + /** + * All the profiles involved in the activation workflow, including the + * parent profile, even if it's not updated. + */ + Builder setProfiles(Collection profiles) { + this.profiles = profiles; + return this; + } + + Builder setActiveRules(Collection activeRules) { + this.activeRules = activeRules; + return this; + } + + Builder setActiveRuleParams(Collection activeRuleParams) { + this.activeRuleParams = activeRuleParams; + return this; + } + + RuleActivationContext build() { + checkArgument(date > 0, "date is not set"); + requireNonNull(baseRulesProfile, "baseRulesProfile is null"); + requireNonNull(rules, "rules is null"); + requireNonNull(ruleParams, "ruleParams is null"); + requireNonNull(profiles, "profiles is null"); + requireNonNull(activeRules, "activeRules is null"); + requireNonNull(activeRuleParams, "activeRuleParams is null"); + return new RuleActivationContext(this); + } + } + + static final class RuleWrapper { + private final RuleDefinitionDto rule; + private final Map paramsByKey; + + private RuleWrapper(RuleDefinitionDto rule, Collection params) { + this.rule = rule; + this.paramsByKey = params.stream().collect(uniqueIndex(RuleParamDto::getName)); + } + + RuleDefinitionDto get() { + return rule; + } + + Collection getParams() { + return paramsByKey.values(); + } + + @CheckForNull + RuleParamDto getParam(String key) { + return paramsByKey.get(key); + } + + @CheckForNull + String getParamDefaultValue(String key) { + RuleParamDto param = getParam(key); + return param != null ? param.getDefaultValue() : null; + } + } + + static final class ActiveRuleWrapper { + private final ActiveRuleDto activeRule; + private final Map paramsByKey; + + private ActiveRuleWrapper(ActiveRuleDto activeRule, Collection params) { + this.activeRule = activeRule; + this.paramsByKey = params.stream().collect(uniqueIndex(ActiveRuleParamDto::getKey)); + } + + ActiveRuleDto get() { + return activeRule; + } + + Collection getParams() { + return paramsByKey.values(); + } + + @CheckForNull + ActiveRuleParamDto getParam(String key) { + return paramsByKey.get(key); + } + + @CheckForNull + String getParamValue(String key) { + ActiveRuleParamDto param = paramsByKey.get(key); + return param != null ? param.getValue() : null; + } + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java index 8ccc75b07a2..3786ccdf433 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java @@ -21,16 +21,20 @@ package org.sonar.server.qualityprofile; import com.google.common.base.Splitter; import java.util.ArrayList; -import java.util.Collections; -import java.util.Iterator; +import java.util.Collection; +import java.util.Date; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import org.apache.commons.lang.StringUtils; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.RuleStatus; import org.sonar.api.server.ServerSide; import org.sonar.api.server.rule.RuleParamType; import org.sonar.api.utils.System2; +import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.qualityprofile.ActiveRuleDao; @@ -41,10 +45,8 @@ import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.qualityprofile.RulesProfileDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleParamDto; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; -import org.sonar.server.rule.index.RuleIndex; -import org.sonar.server.rule.index.RuleQuery; +import org.sonar.server.qualityprofile.RuleActivationContext.ActiveRuleWrapper; +import org.sonar.server.qualityprofile.RuleActivationContext.RuleWrapper; import org.sonar.server.user.UserSession; import org.sonar.server.util.TypeValidations; @@ -60,78 +62,65 @@ public class RuleActivator { private final System2 system2; private final DbClient db; private final TypeValidations typeValidations; - private final RuleActivatorContextFactory contextFactory; - private final RuleIndex ruleIndex; - private final ActiveRuleIndexer activeRuleIndexer; private final UserSession userSession; - public RuleActivator(System2 system2, DbClient db, RuleIndex ruleIndex, - RuleActivatorContextFactory contextFactory, TypeValidations typeValidations, - ActiveRuleIndexer activeRuleIndexer, UserSession userSession) { + public RuleActivator(System2 system2, DbClient db, TypeValidations typeValidations, UserSession userSession) { this.system2 = system2; this.db = db; - this.ruleIndex = ruleIndex; - this.contextFactory = contextFactory; this.typeValidations = typeValidations; - this.activeRuleIndexer = activeRuleIndexer; this.userSession = userSession; } - public List activateOnBuiltInRulesProfile(DbSession dbSession, RuleActivation activation, RulesProfileDto rulesProfile) { - checkArgument(rulesProfile.isBuiltIn(), "Rules profile must be a built-in profile: " + rulesProfile.getKee()); - RuleActivatorContext context = contextFactory.createForBuiltIn(dbSession, activation.getRuleKey(), rulesProfile); + public List activate(DbSession dbSession, RuleActivation activation, RuleActivationContext context) { + context.reset(activation.getRuleKey()); return doActivate(dbSession, activation, context); } - public void activateAndCommit(DbSession dbSession, RuleActivation activation, QProfileDto profile) { - List changes = activate(dbSession, activation, profile); - activeRuleIndexer.commitAndIndex(dbSession, changes); - } - - public List activate(DbSession dbSession, RuleActivation activation, QProfileDto profile) { - RuleActivatorContext context = contextFactory.create(dbSession, activation.getRuleKey(), profile, false); - return doActivate(dbSession, activation, context); - } + private List doActivate(DbSession dbSession, RuleActivation activation, RuleActivationContext context) { + RuleDefinitionDto rule = context.getRule().get(); + checkRequest(RuleStatus.REMOVED != rule.getStatus(), "Rule was removed: %s", rule.getKey()); + checkRequest(!rule.isTemplate(), "Rule template can't be activated on a Quality profile: %s", rule.getKey()); - private List doActivate(DbSession dbSession, RuleActivation activation, RuleActivatorContext context) { - context.verifyForActivation(); List changes = new ArrayList<>(); ActiveRuleChange change; - boolean stopPropagation = false; + boolean stopCascading = false; - ActiveRuleDto activeRule = context.getActiveRule(); + ActiveRuleWrapper activeRule = context.getActiveRule(); + ActiveRuleKey activeRuleKey = ActiveRuleKey.of(context.getRulesProfile(), rule.getKey()); if (activeRule == null) { if (activation.isReset()) { // ignore reset when rule is not activated return changes; } // new activation - change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, context.getActiveRuleKey()); + change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, activeRuleKey); applySeverityAndParamToChange(activation, context, change); - if (context.isCascade() || context.isSameAsParent(change)) { + if (context.isCascading() || isSameAsParent(change, context)) { change.setInheritance(ActiveRule.Inheritance.INHERITED); } } else { // already activated - if (context.isCascade() && activeRule.doesOverride()) { + if (context.isCascading() && activeRule.get().doesOverride()) { // propagating to descendants, but child profile already overrides rule -> stop propagation return changes; } - change = new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, context.getActiveRuleKey()); - if (context.isCascade() && activeRule.getInheritance() == null) { + change = new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activeRuleKey); + if (context.isCascading() && activeRule.get().getInheritance() == null) { // activate on child, then on parent -> mark child as overriding parent change.setInheritance(ActiveRule.Inheritance.OVERRIDES); - change.setSeverity(context.getActiveSeverityBeforeChange()); - change.setParameters(context.activeRuleParamsAsStringMap()); - stopPropagation = true; + change.setSeverity(activeRule.get().getSeverityString()); + for (ActiveRuleParamDto activeParam : activeRule.getParams()) { + change.setParameter(activeParam.getKey(), activeParam.getValue()); + } + stopCascading = true; } else { applySeverityAndParamToChange(activation, context, change); - if (!context.isCascade() && context.getParentActiveRule() != null) { + if (!context.isCascading() && context.getParentActiveRule() != null) { // override rule which is already declared on parents - change.setInheritance(context.isSameAsParent(change) ? ActiveRule.Inheritance.INHERITED : ActiveRule.Inheritance.OVERRIDES); + change.setInheritance(isSameAsParent(change, context) ? ActiveRule.Inheritance.INHERITED : ActiveRule.Inheritance.OVERRIDES); } } - if (context.isSame(change)) { + if (isSame(change, activeRule)) { change = null; } } @@ -141,8 +130,8 @@ public class RuleActivator { persist(change, context, dbSession); } - if (!stopPropagation) { - changes.addAll(cascadeActivation(dbSession, activation, context)); + if (!stopCascading) { + changes.addAll(propagateActivationToDescendants(dbSession, activation, context)); } if (!changes.isEmpty()) { @@ -151,18 +140,19 @@ public class RuleActivator { return changes; } - private void updateProfileDates(DbSession dbSession, RuleActivatorContext context) { - QProfileDto profile = context.getOrganizationProfile(); + private void updateProfileDates(DbSession dbSession, RuleActivationContext context) { + QProfileDto profile = context.getProfile(); if (profile != null) { - profile.setRulesUpdatedAtAsDate(context.getDate()); + profile.setRulesUpdatedAtAsDate(new Date(context.getDate())); if (userSession.isLoggedIn()) { - profile.setUserUpdatedAt(context.getDate().getTime()); + profile.setUserUpdatedAt(context.getDate()); } db.qualityProfileDao().update(dbSession, profile); + } else { // built-in profile, change rules_profiles.rules_updated_at RulesProfileDto rulesProfile = context.getRulesProfile(); - rulesProfile.setRulesUpdatedAtAsDate(context.getDate()); + rulesProfile.setRulesUpdatedAtAsDate(new Date(context.getDate())); db.qualityProfileDao().update(dbSession, rulesProfile); } } @@ -175,88 +165,78 @@ public class RuleActivator { *

* On custom rules, it's always rule parameters that are used */ - private void applySeverityAndParamToChange(RuleActivation request, RuleActivatorContext context, ActiveRuleChange change) { + private void applySeverityAndParamToChange(RuleActivation request, RuleActivationContext context, ActiveRuleChange change) { + RuleWrapper rule = context.getRule(); + ActiveRuleWrapper activeRule = context.getActiveRule(); + ActiveRuleWrapper parentActiveRule = context.getParentActiveRule(); + if (request.isReset()) { // load severity and params from parent profile, else from default values change.setSeverity(firstNonNull( - context.getActiveParentSeverity(), context.getRule().getSeverityString())); - for (RuleParamDto ruleParamDto : context.getRuleParams()) { + parentActiveRule != null ? parentActiveRule.get().getSeverityString() : null, + rule.get().getSeverityString())); + + for (RuleParamDto ruleParamDto : rule.getParams()) { String paramKey = ruleParamDto.getName(); change.setParameter(paramKey, validateParam(ruleParamDto, firstNonNull( - context.getActiveParentParamValue(paramKey), context.getDefaultParamValue(paramKey)))); + parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null, + rule.getParamDefaultValue(paramKey)))); } - } else if (context.getActiveRule() != null) { + } else if (activeRule != null) { // already activated -> load severity and parameters from request, else keep existing ones, else from parent, // else from default change.setSeverity(firstNonNull( request.getSeverity(), - context.getActiveSeverityBeforeChange(), - context.getActiveParentSeverity(), - context.getRule().getSeverityString())); - for (RuleParamDto ruleParamDto : context.getRuleParams()) { + activeRule.get().getSeverityString(), + parentActiveRule != null ? parentActiveRule.get().getSeverityString() : null, + rule.get().getSeverityString())); + for (RuleParamDto ruleParamDto : rule.getParams()) { String paramKey = ruleParamDto.getName(); + String parentValue = parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null; String paramValue = context.hasRequestedParamValue(request, paramKey) ? // If the request contains the parameter then we're using either value from request, or parent value, or default value firstNonNull( context.getRequestedParamValue(request, paramKey), - context.getActiveParentParamValue(paramKey), - context.getDefaultParamValue(paramKey)) + parentValue, + rule.getParamDefaultValue(paramKey)) // If the request doesn't contain the parameter, then we're using either value in DB, or parent value, or default value : firstNonNull( - context.getActiveParamValue(paramKey), - context.getActiveParentParamValue(paramKey), - context.getDefaultParamValue(paramKey)); + activeRule.getParamValue(paramKey), + parentValue, + rule.getParamDefaultValue(paramKey)); change.setParameter(paramKey, validateParam(ruleParamDto, paramValue)); } - } else if (context.getActiveRule() == null) { + } else { // not activated -> load severity and parameters from request, else from parent, else from defaults change.setSeverity(firstNonNull( request.getSeverity(), - context.getActiveParentSeverity(), - context.getRule().getSeverityString())); - for (RuleParamDto ruleParamDto : context.getRuleParams()) { + parentActiveRule != null ? parentActiveRule.get().getSeverityString() : null, + rule.get().getSeverityString())); + for (RuleParamDto ruleParamDto : rule.getParams()) { String paramKey = ruleParamDto.getName(); change.setParameter(paramKey, validateParam(ruleParamDto, firstNonNull( context.getRequestedParamValue(request, paramKey), - context.getActiveParentParamValue(paramKey), - context.getDefaultParamValue(paramKey)))); - } - } - } - - @CheckForNull - private static String firstNonNull(String... strings) { - for (String s : strings) { - if (s != null) { - return s; + parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null, + rule.getParamDefaultValue(paramKey)))); } } - return null; } - private List cascadeActivation(DbSession dbSession, RuleActivation activation, RuleActivatorContext context) { + private List propagateActivationToDescendants(DbSession dbSession, RuleActivation activation, RuleActivationContext context) { List changes = new ArrayList<>(); // get all inherited profiles - getChildren(dbSession, context).forEach(child -> { - RuleActivatorContext childContext = contextFactory.create(dbSession, activation.getRuleKey(), child, true); - changes.addAll(doActivate(dbSession, activation, childContext)); + context.getChildProfiles().forEach(child -> { + context.switchToChild(child); + changes.addAll(doActivate(dbSession, activation, context)); }); return changes; } - protected List getChildren(DbSession session, RuleActivatorContext context) { - QProfileDto orgProfile = context.getOrganizationProfile(); - if (orgProfile != null) { - return db.qualityProfileDao().selectChildren(session, orgProfile); - } - return db.qualityProfileDao().selectChildrenOfBuiltInRulesProfile(session, context.getRulesProfile()); - } - - private void persist(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) { + private void persist(ActiveRuleChange change, RuleActivationContext context, DbSession dbSession) { ActiveRuleDto activeRule = null; if (change.getType() == ActiveRuleChange.Type.ACTIVATED) { activeRule = doInsert(change, context, dbSession); @@ -271,12 +251,14 @@ public class RuleActivator { db.qProfileChangeDao().insert(dbSession, change.toDto(userSession.getLogin())); } - private ActiveRuleDto doInsert(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) { + private ActiveRuleDto doInsert(ActiveRuleChange change, RuleActivationContext context, DbSession dbSession) { ActiveRuleDao dao = db.activeRuleDao(); + RuleWrapper rule = context.getRule(); + ActiveRuleDto activeRule = new ActiveRuleDto(); activeRule.setProfileId(context.getRulesProfile().getId()); - activeRule.setRuleId(context.getRule().getId()); - activeRule.setKey(ActiveRuleKey.of(context.getRulesProfile(), context.getRule().getKey())); + activeRule.setRuleId(rule.get().getId()); + activeRule.setKey(ActiveRuleKey.of(context.getRulesProfile(), rule.get().getKey())); String severity = change.getSeverity(); if (severity != null) { activeRule.setSeverity(severity); @@ -290,7 +272,7 @@ public class RuleActivator { dao.insert(dbSession, activeRule); for (Map.Entry param : change.getParameters().entrySet()) { if (param.getValue() != null) { - ActiveRuleParamDto paramDto = ActiveRuleParamDto.createFor(context.getRuleParam(param.getKey())); + ActiveRuleParamDto paramDto = ActiveRuleParamDto.createFor(rule.getParam(param.getKey())); paramDto.setValue(param.getValue()); dao.insertParam(dbSession, activeRule, paramDto); } @@ -298,108 +280,66 @@ public class RuleActivator { return activeRule; } - private ActiveRuleDto doUpdate(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) { + private ActiveRuleDto doUpdate(ActiveRuleChange change, RuleActivationContext context, DbSession dbSession) { + ActiveRuleWrapper activeRule = context.getActiveRule(); + if (activeRule == null) { + return null; + } ActiveRuleDao dao = db.activeRuleDao(); - ActiveRuleDto activeRule = context.getActiveRule(); - if (activeRule != null) { - String severity = change.getSeverity(); - if (severity != null) { - activeRule.setSeverity(severity); - } - ActiveRule.Inheritance inheritance = change.getInheritance(); - if (inheritance != null) { - activeRule.setInheritance(inheritance.name()); - } - activeRule.setUpdatedAt(system2.now()); - dao.update(dbSession, activeRule); - - for (Map.Entry param : change.getParameters().entrySet()) { - ActiveRuleParamDto activeRuleParamDto = context.getActiveParamBeforeChange(param.getKey()); - if (activeRuleParamDto == null) { - // did not exist - if (param.getValue() != null) { - activeRuleParamDto = ActiveRuleParamDto.createFor(context.getRuleParam(param.getKey())); - activeRuleParamDto.setValue(param.getValue()); - dao.insertParam(dbSession, activeRule, activeRuleParamDto); - } + String severity = change.getSeverity(); + if (severity != null) { + activeRule.get().setSeverity(severity); + } + ActiveRule.Inheritance inheritance = change.getInheritance(); + if (inheritance != null) { + activeRule.get().setInheritance(inheritance.name()); + } + activeRule.get().setUpdatedAt(system2.now()); + dao.update(dbSession, activeRule.get()); + + for (Map.Entry param : change.getParameters().entrySet()) { + ActiveRuleParamDto activeRuleParamDto = activeRule.getParam(param.getKey()); + if (activeRuleParamDto == null) { + // did not exist + if (param.getValue() != null) { + activeRuleParamDto = ActiveRuleParamDto.createFor(context.getRule().getParam(param.getKey())); + activeRuleParamDto.setValue(param.getValue()); + dao.insertParam(dbSession, activeRule.get(), activeRuleParamDto); + } + } else { + if (param.getValue() != null) { + activeRuleParamDto.setValue(param.getValue()); + dao.updateParam(dbSession, activeRuleParamDto); } else { - if (param.getValue() != null) { - activeRuleParamDto.setValue(param.getValue()); - dao.updateParam(dbSession, activeRuleParamDto); - } else { - dao.deleteParam(dbSession, activeRuleParamDto); - } + dao.deleteParam(dbSession, activeRuleParamDto); } } } - return activeRule; - } - - /** - * Deactivate a rule on a Quality profile. Does nothing if the rule is not activated, but - * fails (fast) if the rule or the profile does not exist. - */ - public void deactivateAndCommit(DbSession dbSession, QProfileDto profile, RuleKey ruleKey) { - List changes = deactivate(dbSession, profile, ruleKey); - activeRuleIndexer.commitAndIndex(dbSession, changes); - } - - /** - * Deactivate a rule on a Quality profile WITHOUT committing db session and WITHOUT checking permissions - */ - List deactivate(DbSession dbSession, QProfileDto profile, RuleKey ruleKey) { - return deactivate(dbSession, profile, ruleKey, false); - } - - /** - * Deletes a rule from all Quality profiles. - */ - public List delete(DbSession dbSession, RuleDefinitionDto rule) { - List changes = new ArrayList<>(); - List activeRuleIds = new ArrayList<>(); - - db.activeRuleDao().selectByRuleIdOfAllOrganizations(dbSession, rule.getId()).forEach(ar -> { - activeRuleIds.add(ar.getId()); - changes.add(new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, ar)); - }); - - db.activeRuleDao().deleteByIds(dbSession, activeRuleIds); - db.activeRuleDao().deleteParamsByActiveRuleIds(dbSession, activeRuleIds); - - return changes; + return activeRule.get(); } - /** - * @param force if true then inherited rules are deactivated - */ - public List deactivate(DbSession dbSession, QProfileDto profile, RuleKey ruleKey, boolean force) { - RuleActivatorContext context = contextFactory.create(dbSession, ruleKey, profile, false); - return cascadeDeactivation(dbSession, context, ruleKey, force); + public List deactivate(DbSession dbSession, RuleActivationContext context, RuleKey ruleKey, boolean force) { + context.reset(ruleKey); + return doDeactivate(dbSession, context, force); } - public List deactivateOnBuiltInRulesProfile(DbSession dbSession, RulesProfileDto rulesProfile, RuleKey ruleKey, boolean force) { - checkArgument(rulesProfile.isBuiltIn(), "Rules profile must be a built-in profile: " + rulesProfile.getKee()); - RuleActivatorContext context = contextFactory.createForBuiltIn(dbSession, ruleKey, rulesProfile); - return cascadeDeactivation(dbSession, context, ruleKey, force); - } - - private List cascadeDeactivation(DbSession dbSession, RuleActivatorContext context, RuleKey ruleKey, boolean force) { + private List doDeactivate(DbSession dbSession, RuleActivationContext context, boolean force) { List changes = new ArrayList<>(); - ActiveRuleChange change; - ActiveRuleDto activeRuleDto = context.getActiveRule(); - if (activeRuleDto == null) { + ActiveRuleWrapper activeRule = context.getActiveRule(); + if (activeRule == null) { return changes; } - checkRequest(force || context.isCascade() || activeRuleDto.getInheritance() == null, "Cannot deactivate inherited rule '%s'", ruleKey); - change = new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, activeRuleDto); + + ActiveRuleChange change; + checkRequest(force || context.isCascading() || activeRule.get().getInheritance() == null, "Cannot deactivate inherited rule '%s'", context.getRule().get().getKey()); + change = new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, activeRule.get()); changes.add(change); persist(change, context, dbSession); // get all inherited profiles (they are not built-in by design) - - getChildren(dbSession, context).forEach(child -> { - RuleActivatorContext childContext = contextFactory.create(dbSession, ruleKey, child, true); - changes.addAll(cascadeDeactivation(dbSession, childContext, ruleKey, force)); + context.getChildProfiles().forEach(child -> { + context.switchToChild(child); + changes.addAll(doDeactivate(dbSession, context, force)); }); if (!changes.isEmpty()) { @@ -423,118 +363,119 @@ public class RuleActivator { return value; } - public BulkChangeResult bulkActivateAndCommit(DbSession dbSession, RuleQuery ruleQuery, QProfileDto profile, @Nullable String severity) { - BulkChangeResult result = new BulkChangeResult(); - Iterator rules = ruleIndex.searchAll(ruleQuery); - while (rules.hasNext()) { - RuleKey ruleKey = rules.next(); - try { - RuleActivation activation = RuleActivation.create(ruleKey, severity, null); - List changes = activate(dbSession, activation, profile); - result.addChanges(changes); - if (!changes.isEmpty()) { - result.incrementSucceeded(); - } + public RuleActivationContext createContextForBuiltInProfile(DbSession dbSession, RulesProfileDto builtInProfile, Collection ruleKeys) { + checkArgument(builtInProfile.isBuiltIn(), "Rules profile with UUID %s is not built-in", builtInProfile.getKee()); - } catch (BadRequestException e) { - // other exceptions stop the bulk activation - result.incrementFailed(); - result.getErrors().addAll(e.errors()); - } + RuleActivationContext.Builder builder = new RuleActivationContext.Builder(); + + // load rules + List rules = completeWithRules(dbSession, builder, ruleKeys); + + // load profiles + List aliasedBuiltInProfiles = db.qualityProfileDao().selectQProfilesByRuleProfile(dbSession, builtInProfile); + List profiles = new ArrayList<>(aliasedBuiltInProfiles); + for (QProfileDto aliasProfile : aliasedBuiltInProfiles) { + // FIXME N+1 syndrome. To be optimized by replacing db column parent_kee by ancestor_keys. + profiles.addAll(db.qualityProfileDao().selectDescendants(dbSession, aliasProfile)); } - activeRuleIndexer.commitAndIndex(dbSession, result.getChanges()); - return result; + builder.setProfiles(profiles); + builder.setBaseProfile(builtInProfile); + + // load active rules + Collection ruleProfileUuids = Stream + .concat(Stream.of(builtInProfile.getKee()), profiles.stream().map(QProfileDto::getRulesProfileUuid)) + .collect(MoreCollectors.toHashSet(profiles.size() + 1)); + completeWithActiveRules(dbSession, builder, rules, ruleProfileUuids); + + return builder.build(); } - public BulkChangeResult bulkDeactivateAndCommit(DbSession dbSession, RuleQuery ruleQuery, QProfileDto profile) { - BulkChangeResult result = new BulkChangeResult(); - Iterator rules = ruleIndex.searchAll(ruleQuery); - while (rules.hasNext()) { - try { - RuleKey ruleKey = rules.next(); - List changes = deactivate(dbSession, profile, ruleKey); - result.addChanges(changes); - if (!changes.isEmpty()) { - result.incrementSucceeded(); - } - } catch (BadRequestException e) { - // other exceptions stop the bulk activation - result.incrementFailed(); - result.getErrors().addAll(e.errors()); - } + public RuleActivationContext createContextForUserProfile(DbSession dbSession, QProfileDto profile, Collection ruleKeys) { + checkArgument(!profile.isBuiltIn(), "Profile with UUID %s is built-in", profile.getKee()); + RuleActivationContext.Builder builder = new RuleActivationContext.Builder(); + + // load rules + List rules = completeWithRules(dbSession, builder, ruleKeys); + + // load descendant profiles + List profiles = new ArrayList<>(db.qualityProfileDao().selectDescendants(dbSession, profile)); + profiles.add(profile); + if (profile.getParentKee() != null) { + profiles.add(db.qualityProfileDao().selectByUuid(dbSession, profile.getParentKee())); } - activeRuleIndexer.commitAndIndex(dbSession, result.getChanges()); - return result; - } + builder.setProfiles(profiles); + builder.setBaseProfile(profile); - public List setParentAndCommit(DbSession dbSession, QProfileDto profile, @Nullable QProfileDto parent) { - checkRequest( - parent == null || profile.getLanguage().equals(parent.getLanguage()), - "Cannot set the profile '%s' as the parent of profile '%s' since their languages differ ('%s' != '%s')", - parent != null ? parent.getKee() : "", profile.getKee(), parent != null ? parent.getLanguage() : "", profile.getLanguage()); + // load active rules + Collection ruleProfileUuids = profiles.stream() + .map(QProfileDto::getRulesProfileUuid) + .collect(MoreCollectors.toHashSet(profiles.size())); + completeWithActiveRules(dbSession, builder, rules, ruleProfileUuids); - List changes = new ArrayList<>(); - if (parent == null) { - // unset if parent is defined, else nothing to do - changes.addAll(removeParent(dbSession, profile)); + return builder.build(); + } - } else if (profile.getParentKee() == null || !parent.getKee().equals(profile.getParentKee())) { - checkRequest(!isDescendant(dbSession, profile, parent), "Descendant profile '%s' can not be selected as parent of '%s'", parent.getKee(), profile.getKee()); - changes.addAll(removeParent(dbSession, profile)); + private List completeWithRules(DbSession dbSession, RuleActivationContext.Builder builder, Collection ruleKeys) { + List rules = db.ruleDao().selectDefinitionByKeys(dbSession, ruleKeys); + builder.setRules(rules); + builder.setRuleParams(db.ruleDao().selectRuleParamsByRuleKeys(dbSession, ruleKeys)); + return rules; + } - // set new parent - profile.setParentKee(parent.getKee()); - db.qualityProfileDao().update(dbSession, profile); - for (ActiveRuleDto parentActiveRule : db.activeRuleDao().selectByProfile(dbSession, parent)) { - try { - RuleActivation activation = RuleActivation.create(parentActiveRule.getRuleKey(), null, null); - changes.addAll(activate(dbSession, activation, profile)); - } catch (BadRequestException e) { - // for example because rule status is REMOVED - // TODO return errors - } + private void completeWithActiveRules(DbSession dbSession, RuleActivationContext.Builder builder, Collection rules, Collection ruleProfileUuids) { + Collection activeRules = db.activeRuleDao().selectByRulesAndRuleProfileUuids(dbSession, rules, ruleProfileUuids); + builder.setActiveRules(activeRules); + List activeRuleIds = activeRules.stream().map(ActiveRuleDto::getId).collect(MoreCollectors.toArrayList(activeRules.size())); + builder.setActiveRuleParams(db.activeRuleDao().selectParamsByActiveRuleIds(dbSession, activeRuleIds)); + } + + private static boolean isSame(ActiveRuleChange change, ActiveRuleWrapper activeRule) { + ActiveRule.Inheritance inheritance = change.getInheritance(); + if (inheritance != null && !inheritance.name().equals(activeRule.get().getInheritance())) { + return false; + } + String severity = change.getSeverity(); + if (severity != null && !severity.equals(activeRule.get().getSeverityString())) { + return false; + } + for (Map.Entry changeParam : change.getParameters().entrySet()) { + String activeParamValue = activeRule.getParamValue(changeParam.getKey()); + if (changeParam.getValue() == null && activeParamValue != null) { + return false; + } + if (changeParam.getValue() != null && (activeParamValue == null || !StringUtils.equals(changeParam.getValue(), activeParamValue))) { + return false; } } - activeRuleIndexer.commitAndIndex(dbSession, changes); - return changes; + return true; } /** - * Does not commit + * True if trying to override an inherited rule but with exactly the same values */ - private List removeParent(DbSession dbSession, QProfileDto profile) { - if (profile.getParentKee() != null) { - List changes = new ArrayList<>(); - profile.setParentKee(null); - db.qualityProfileDao().update(dbSession, profile); - for (ActiveRuleDto activeRule : db.activeRuleDao().selectByProfile(dbSession, profile)) { - if (ActiveRuleDto.INHERITED.equals(activeRule.getInheritance())) { - changes.addAll(deactivate(dbSession, profile, activeRule.getRuleKey(), true)); - } else if (ActiveRuleDto.OVERRIDES.equals(activeRule.getInheritance())) { - activeRule.setInheritance(null); - activeRule.setUpdatedAt(system2.now()); - db.activeRuleDao().update(dbSession, activeRule); - changes.add(new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activeRule).setInheritance(null)); - } + private static boolean isSameAsParent(ActiveRuleChange change, RuleActivationContext context) { + ActiveRuleWrapper parentActiveRule = context.getParentActiveRule(); + if (parentActiveRule == null) { + return false; + } + if (!StringUtils.equals(change.getSeverity(), parentActiveRule.get().getSeverityString())) { + return false; + } + for (Map.Entry entry : change.getParameters().entrySet()) { + if (entry.getValue() != null && !entry.getValue().equals(parentActiveRule.getParamValue(entry.getKey()))) { + return false; } - return changes; } - return Collections.emptyList(); + return true; } - private boolean isDescendant(DbSession dbSession, QProfileDto childProfile, @Nullable QProfileDto parentProfile) { - QProfileDto currentParent = parentProfile; - while (currentParent != null) { - if (childProfile.getName().equals(currentParent.getName())) { - return true; - } - String parentKey = currentParent.getParentKee(); - if (parentKey != null) { - currentParent = db.qualityProfileDao().selectByUuid(dbSession, parentKey); - } else { - currentParent = null; + @CheckForNull + private static String firstNonNull(String... strings) { + for (String s : strings) { + if (s != null) { + return s; } } - return false; + return null; } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java deleted file mode 100644 index c14bfb3d47a..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java +++ /dev/null @@ -1,264 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 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.server.qualityprofile; - -import com.google.common.collect.Maps; -import java.util.Collection; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; -import javax.annotation.CheckForNull; -import javax.annotation.Nullable; -import org.apache.commons.lang.StringUtils; -import org.sonar.api.rule.RuleStatus; -import org.sonar.db.qualityprofile.ActiveRuleDto; -import org.sonar.db.qualityprofile.ActiveRuleKey; -import org.sonar.db.qualityprofile.ActiveRuleParamDto; -import org.sonar.db.qualityprofile.QProfileDto; -import org.sonar.db.qualityprofile.RulesProfileDto; -import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.db.rule.RuleParamDto; - -import static com.google.common.base.Preconditions.checkArgument; -import static org.sonar.server.ws.WsUtils.checkRequest; - -class RuleActivatorContext { - - private final QProfileDto profile; - private final RulesProfileDto rulesProfile; - private final Date now = new Date(); - private RuleDefinitionDto rule; - private final Map ruleParamsByKey = new HashMap<>(); - private ActiveRuleDto activeRule; - private ActiveRuleDto parentActiveRule; - private final Map activeRuleParams = new HashMap<>(); - private final Map parentActiveRuleParams = new HashMap<>(); - private final boolean isCascade; - - RuleActivatorContext(QProfileDto profile, boolean isCascade) { - this.profile = profile; - this.rulesProfile = RulesProfileDto.from(profile); - this.isCascade = isCascade; - } - - RuleActivatorContext(RulesProfileDto rulesProfile) { - checkArgument(rulesProfile.isBuiltIn(), "Rules profile must be a built-in profile: " + rulesProfile.getKee()); - this.profile = null; - this.rulesProfile = rulesProfile; - this.isCascade = false; - } - - /** - * The organization profile that is being updated. Null if updating built-in profile. - */ - @CheckForNull - QProfileDto getOrganizationProfile() { - return profile; - } - - RulesProfileDto getRulesProfile() { - return rulesProfile; - } - - /** - * Whether activation is being applied on a descendant profile ({@code true}) or - * on the target profile ({@code false}). - */ - boolean isCascade() { - return isCascade; - } - - ActiveRuleKey getActiveRuleKey() { - return ActiveRuleKey.of(rulesProfile, rule.getKey()); - } - - RuleDefinitionDto getRule() { - return rule; - } - - RuleActivatorContext setRule(RuleDefinitionDto rule) { - this.rule = rule; - return this; - } - - /** - * Date of request for activation. - */ - Date getDate() { - return now; - } - - @CheckForNull - RuleParamDto getRuleParam(String key) { - return ruleParamsByKey.get(key); - } - - Collection getRuleParams() { - return ruleParamsByKey.values(); - } - - void setRuleParams(Collection ruleParams) { - this.ruleParamsByKey.clear(); - for (RuleParamDto ruleParam : ruleParams) { - this.ruleParamsByKey.put(ruleParam.getName(), ruleParam); - } - } - - /** - * The active rule as it was before applying the current activation. - * Return null if the rule was not activated. - */ - @CheckForNull - ActiveRuleDto getActiveRule() { - return activeRule; - } - - RuleActivatorContext setActiveRule(@Nullable ActiveRuleDto a) { - this.activeRule = a; - return this; - } - - @CheckForNull - ActiveRuleDto getParentActiveRule() { - return parentActiveRule; - } - - void setParentActiveRule(@Nullable ActiveRuleDto a) { - this.parentActiveRule = a; - } - - @CheckForNull - String getRequestedParamValue(RuleActivation request, String key) { - if (rule.isCustomRule()) { - return null; - } - return request.getParameter(key); - } - - boolean hasRequestedParamValue(RuleActivation request, String key) { - return request.hasParameter(key); - } - - @CheckForNull - String getActiveParamValue(String key) { - ActiveRuleParamDto param = activeRuleParams.get(key); - return param != null ? param.getValue() : null; - } - - @CheckForNull - ActiveRuleParamDto getActiveParamBeforeChange(String key) { - return activeRuleParams.get(key); - } - - @CheckForNull - String getActiveParentParamValue(String key) { - ActiveRuleParamDto param = parentActiveRuleParams.get(key); - return param != null ? param.getValue() : null; - } - - @CheckForNull - String getDefaultParamValue(String key) { - RuleParamDto param = ruleParamsByKey.get(key); - return param != null ? param.getDefaultValue() : null; - } - - @CheckForNull - String getActiveSeverityBeforeChange() { - return activeRule != null ? activeRule.getSeverityString() : null; - } - - @CheckForNull - String getActiveParentSeverity() { - return parentActiveRule != null ? parentActiveRule.getSeverityString() : null; - } - - - Map activeRuleParamsAsStringMap() { - Map params = Maps.newHashMap(); - for (Map.Entry param : activeRuleParams.entrySet()) { - params.put(param.getKey(), param.getValue().getValue()); - } - return params; - } - - void setActiveRuleParams(@Nullable Collection a) { - activeRuleParams.clear(); - if (a != null) { - for (ActiveRuleParamDto ar : a) { - this.activeRuleParams.put(ar.getKey(), ar); - } - } - } - - void setParentActiveRuleParams(@Nullable Collection a) { - parentActiveRuleParams.clear(); - if (a != null) { - for (ActiveRuleParamDto ar : a) { - this.parentActiveRuleParams.put(ar.getKey(), ar); - } - } - } - - /** - * True if trying to override an inherited rule but with exactly the same values - */ - boolean isSameAsParent(ActiveRuleChange change) { - if (parentActiveRule == null) { - return false; - } - if (!StringUtils.equals(change.getSeverity(), parentActiveRule.getSeverityString())) { - return false; - } - for (Map.Entry entry : change.getParameters().entrySet()) { - if (entry.getValue() != null && !entry.getValue().equals(getActiveParentParamValue(entry.getKey()))) { - return false; - } - } - return true; - } - - boolean isSame(ActiveRuleChange change) { - ActiveRule.Inheritance inheritance = change.getInheritance(); - if (inheritance != null && !inheritance.name().equals(activeRule.getInheritance())) { - return false; - } - String severity = change.getSeverity(); - if (severity != null && !severity.equals(activeRule.getSeverityString())) { - return false; - } - for (Map.Entry changeParam : change.getParameters().entrySet()) { - ActiveRuleParamDto param = activeRuleParams.get(changeParam.getKey()); - if (changeParam.getValue() == null && param != null && param.getValue() != null) { - return false; - } - if (changeParam.getValue() != null && (param == null || !StringUtils.equals(changeParam.getValue(), param.getValue()))) { - return false; - } - } - return true; - } - - void verifyForActivation() { - checkRequest(RuleStatus.REMOVED != rule.getStatus(), "Rule was removed: %s", rule.getKey()); - checkRequest(!rule.isTemplate(), "Rule template can't be activated on a Quality profile: %s", rule.getKey()); - checkRequest(rulesProfile.getLanguage().equals(rule.getLanguage()), - "Rule %s and profile %s have different languages", rule.getKey(), profile != null ? profile.getKee() : rulesProfile.getKee()); - } -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java deleted file mode 100644 index 044a9e0f432..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java +++ /dev/null @@ -1,113 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 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.server.qualityprofile; - -import java.util.Collection; -import java.util.List; -import java.util.Optional; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.server.ServerSide; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.db.qualityprofile.ActiveRuleDto; -import org.sonar.db.qualityprofile.ActiveRuleKey; -import org.sonar.db.qualityprofile.ActiveRuleParamDto; -import org.sonar.db.qualityprofile.QProfileDto; -import org.sonar.db.qualityprofile.RulesProfileDto; -import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.db.rule.RuleParamDto; - -import static org.sonar.server.ws.WsUtils.checkRequest; - -@ServerSide -public class RuleActivatorContextFactory { - - private final DbClient db; - - public RuleActivatorContextFactory(DbClient db) { - this.db = db; - } - - RuleActivatorContext createForBuiltIn(DbSession dbSession, RuleKey ruleKey, RulesProfileDto rulesProfile) { - RuleActivatorContext context = new RuleActivatorContext(rulesProfile); - return init(dbSession, ruleKey, context); - } - - RuleActivatorContext create(DbSession dbSession, RuleKey ruleKey, QProfileDto profile, boolean cascade) { - RuleActivatorContext context = new RuleActivatorContext(profile, cascade); - return init(dbSession, ruleKey, context); - } - - private RuleActivatorContext init(DbSession dbSession, RuleKey ruleKey, RuleActivatorContext context) { - initRule(ruleKey, context, dbSession); - initActiveRules(context.getRulesProfile(), ruleKey, context, dbSession, false); - - QProfileDto orgProfile = context.getOrganizationProfile(); - if (orgProfile != null && orgProfile.getParentKee() != null) { - QProfileDto parent = db.qualityProfileDao().selectByUuid(dbSession, orgProfile.getParentKee()); - if (parent != null) { - initActiveRules(RulesProfileDto.from(parent), ruleKey, context, dbSession, true); - } - } - - return context; - } - - private RuleDefinitionDto initRule(RuleKey ruleKey, RuleActivatorContext context, DbSession dbSession) { - Optional rule = getRule(dbSession, ruleKey); - checkRequest(rule.isPresent(), "Rule not found: %s", ruleKey); - RuleDefinitionDto ruleDefinitionDto = rule.get(); - context.setRule(ruleDefinitionDto); - context.setRuleParams(getRuleParams(dbSession, ruleDefinitionDto)); - return ruleDefinitionDto; - } - - private void initActiveRules(RulesProfileDto rulesProfile, RuleKey ruleKey, RuleActivatorContext context, DbSession dbSession, boolean isParent) { - ActiveRuleKey key = ActiveRuleKey.of(rulesProfile, ruleKey); - Optional activeRule = getActiveRule(dbSession, key); - Collection activeRuleParams = null; - if (activeRule.isPresent()) { - activeRuleParams = getActiveRuleParams(dbSession, activeRule.get()); - } - if (isParent) { - context.setParentActiveRule(activeRule.orElse(null)); - context.setParentActiveRuleParams(activeRuleParams); - } else { - context.setActiveRule(activeRule.orElse(null)); - context.setActiveRuleParams(activeRuleParams); - } - } - - Optional getRule(DbSession dbSession, RuleKey ruleKey) { - return Optional.ofNullable(db.ruleDao().selectDefinitionByKey(dbSession, ruleKey).orElse(null)); - } - - Collection getRuleParams(DbSession dbSession, RuleDefinitionDto ruleDefinitionDto) { - return db.ruleDao().selectRuleParamsByRuleKey(dbSession, ruleDefinitionDto.getKey()); - } - - Optional getActiveRule(DbSession dbSession, ActiveRuleKey key) { - return db.activeRuleDao().selectByKey(dbSession, key); - } - - List getActiveRuleParams(DbSession dbSession, ActiveRuleDto activeRuleDto) { - return db.activeRuleDao().selectParamsByActiveRuleId(dbSession, activeRuleDto.getId()); - } -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java index 30b3bdefa26..1f620be292c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java @@ -30,11 +30,12 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.qualityprofile.RuleActivation; -import org.sonar.server.qualityprofile.RuleActivator; import org.sonar.server.user.UserSession; import static java.lang.String.format; +import static java.util.Collections.singletonList; import static org.sonar.core.util.Uuids.UUID_EXAMPLE_01; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.ACTION_ACTIVATE_RULE; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_KEY; @@ -46,11 +47,11 @@ import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters. public class ActivateRuleAction implements QProfileWsAction { private final DbClient dbClient; - private final RuleActivator ruleActivator; + private final QProfileRules ruleActivator; private final UserSession userSession; private final QProfileWsSupport wsSupport; - public ActivateRuleAction(DbClient dbClient, RuleActivator ruleActivator, UserSession userSession, QProfileWsSupport wsSupport) { + public ActivateRuleAction(DbClient dbClient, QProfileRules ruleActivator, UserSession userSession, QProfileWsSupport wsSupport) { this.dbClient = dbClient; this.ruleActivator = ruleActivator; this.userSession = userSession; @@ -104,7 +105,7 @@ public class ActivateRuleAction implements QProfileWsAction { OrganizationDto organization = wsSupport.getOrganization(dbSession, profile); wsSupport.checkCanEdit(dbSession, organization, profile); RuleActivation activation = readActivation(request); - ruleActivator.activateAndCommit(dbSession, activation, profile); + ruleActivator.activateAndCommit(dbSession, profile, singletonList(activation)); } response.noContent(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java index 6905f07d593..6b132a30187 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java @@ -28,7 +28,7 @@ import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.server.qualityprofile.BulkChangeResult; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.rule.ws.RuleQueryFactory; import org.sonar.server.user.UserSession; @@ -44,14 +44,14 @@ public class ActivateRulesAction implements QProfileWsAction { private final RuleQueryFactory ruleQueryFactory; private final UserSession userSession; - private final RuleActivator ruleActivator; + private final QProfileRules qProfileRules; private final DbClient dbClient; private final QProfileWsSupport wsSupport; - public ActivateRulesAction(RuleQueryFactory ruleQueryFactory, UserSession userSession, RuleActivator ruleActivator, QProfileWsSupport wsSupport, DbClient dbClient) { + public ActivateRulesAction(RuleQueryFactory ruleQueryFactory, UserSession userSession, QProfileRules qProfileRules, QProfileWsSupport wsSupport, DbClient dbClient) { this.ruleQueryFactory = ruleQueryFactory; this.userSession = userSession; - this.ruleActivator = ruleActivator; + this.qProfileRules = qProfileRules; this.dbClient = dbClient; this.wsSupport = wsSupport; } @@ -93,7 +93,7 @@ public class ActivateRulesAction implements QProfileWsAction { OrganizationDto organization = wsSupport.getOrganization(dbSession, profile); wsSupport.checkCanEdit(dbSession, organization, profile); wsSupport.checkNotBuiltInt(profile); - result = ruleActivator.bulkActivateAndCommit(dbSession, ruleQueryFactory.createRuleQuery(dbSession, request), profile, request.param(PARAM_TARGET_SEVERITY)); + result = qProfileRules.bulkActivateAndCommit(dbSession, profile, ruleQueryFactory.createRuleQuery(dbSession, request), request.param(PARAM_TARGET_SEVERITY)); } writeResponse(result, response); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ChangeParentAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ChangeParentAction.java index c4df9d3012a..f50c1380c16 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ChangeParentAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ChangeParentAction.java @@ -28,7 +28,7 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileTree; import org.sonar.server.user.UserSession; import org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters; @@ -40,12 +40,12 @@ import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters. public class ChangeParentAction implements QProfileWsAction { private final DbClient dbClient; - private final RuleActivator ruleActivator; + private final QProfileTree ruleActivator; private final Languages languages; private final QProfileWsSupport wsSupport; private final UserSession userSession; - public ChangeParentAction(DbClient dbClient, RuleActivator ruleActivator, + public ChangeParentAction(DbClient dbClient, QProfileTree ruleActivator, Languages languages, QProfileWsSupport wsSupport, UserSession userSession) { this.dbClient = dbClient; this.ruleActivator = ruleActivator; @@ -96,7 +96,7 @@ public class ChangeParentAction implements QProfileWsAction { String parentKey = request.param(PARAM_PARENT_KEY); String parentName = request.param(QualityProfileWsParameters.PARAM_PARENT_QUALITY_PROFILE); if (isEmpty(parentKey) && isEmpty(parentName)) { - ruleActivator.setParentAndCommit(dbSession, profile, null); + ruleActivator.removeParentAndCommit(dbSession, profile); } else { String parentOrganizationKey = parentKey == null ? organization.getKey() : null; String parentLanguage = parentKey == null ? request.param(PARAM_LANGUAGE) : null; diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java index 7776b99a94d..7ef5fc14741 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRuleAction.java @@ -27,9 +27,10 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.user.UserSession; +import static java.util.Collections.singletonList; import static org.sonar.core.util.Uuids.UUID_EXAMPLE_01; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.ACTION_DEACTIVATE_RULE; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_KEY; @@ -38,11 +39,11 @@ import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters. public class DeactivateRuleAction implements QProfileWsAction { private final DbClient dbClient; - private final RuleActivator ruleActivator; + private final QProfileRules ruleActivator; private final UserSession userSession; private final QProfileWsSupport wsSupport; - public DeactivateRuleAction(DbClient dbClient, RuleActivator ruleActivator, UserSession userSession, QProfileWsSupport wsSupport) { + public DeactivateRuleAction(DbClient dbClient, QProfileRules ruleActivator, UserSession userSession, QProfileWsSupport wsSupport) { this.dbClient = dbClient; this.ruleActivator = ruleActivator; this.userSession = userSession; @@ -84,7 +85,7 @@ public class DeactivateRuleAction implements QProfileWsAction { QProfileDto profile = wsSupport.getProfile(dbSession, QProfileReference.fromKey(qualityProfileKey)); OrganizationDto organization = wsSupport.getOrganization(dbSession, profile); wsSupport.checkCanEdit(dbSession, organization, profile); - ruleActivator.deactivateAndCommit(dbSession, profile, ruleKey); + ruleActivator.deactivateAndCommit(dbSession, profile, singletonList(ruleKey)); } response.noContent(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java index 992a717d474..fd779c15d16 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java @@ -27,7 +27,7 @@ import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.server.qualityprofile.BulkChangeResult; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.rule.ws.RuleQueryFactory; import org.sonar.server.user.UserSession; @@ -42,11 +42,11 @@ public class DeactivateRulesAction implements QProfileWsAction { private final RuleQueryFactory ruleQueryFactory; private final UserSession userSession; - private final RuleActivator ruleActivator; + private final QProfileRules ruleActivator; private final QProfileWsSupport wsSupport; private final DbClient dbClient; - public DeactivateRulesAction(RuleQueryFactory ruleQueryFactory, UserSession userSession, RuleActivator ruleActivator, QProfileWsSupport wsSupport, DbClient dbClient) { + public DeactivateRulesAction(RuleQueryFactory ruleQueryFactory, UserSession userSession, QProfileRules ruleActivator, QProfileWsSupport wsSupport, DbClient dbClient) { this.ruleQueryFactory = ruleQueryFactory; this.userSession = userSession; this.ruleActivator = ruleActivator; @@ -85,7 +85,7 @@ public class DeactivateRulesAction implements QProfileWsAction { QProfileDto profile = wsSupport.getProfile(dbSession, QProfileReference.fromKey(qualityProfileKey)); OrganizationDto organization = wsSupport.getOrganization(dbSession, profile); wsSupport.checkCanEdit(dbSession, organization, profile); - result = ruleActivator.bulkDeactivateAndCommit(dbSession, ruleQueryFactory.createRuleQuery(dbSession, request), profile); + result = ruleActivator.bulkDeactivateAndCommit(dbSession, profile, ruleQueryFactory.createRuleQuery(dbSession, request)); } writeResponse(result, response); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java index a1d3e16a3e1..dcea28c7cb1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java @@ -54,7 +54,7 @@ import org.sonar.db.rule.RuleParamDto; import org.sonar.db.rule.RuleRepositoryDto; import org.sonar.server.organization.OrganizationFlags; import org.sonar.server.qualityprofile.ActiveRuleChange; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndexer; @@ -70,7 +70,7 @@ public class RegisterRules implements Startable { private static final Logger LOG = Loggers.get(RegisterRules.class); private final RuleDefinitionsLoader defLoader; - private final RuleActivator ruleActivator; + private final QProfileRules qProfileRules; private final DbClient dbClient; private final RuleIndexer ruleIndexer; private final ActiveRuleIndexer activeRuleIndexer; @@ -79,11 +79,11 @@ public class RegisterRules implements Startable { private final OrganizationFlags organizationFlags; private final WebServerRuleFinder webServerRuleFinder; - public RegisterRules(RuleDefinitionsLoader defLoader, RuleActivator ruleActivator, DbClient dbClient, RuleIndexer ruleIndexer, + public RegisterRules(RuleDefinitionsLoader defLoader, QProfileRules qProfileRules, DbClient dbClient, RuleIndexer ruleIndexer, ActiveRuleIndexer activeRuleIndexer, Languages languages, System2 system2, OrganizationFlags organizationFlags, WebServerRuleFinder webServerRuleFinder) { this.defLoader = defLoader; - this.ruleActivator = ruleActivator; + this.qProfileRules = qProfileRules; this.dbClient = dbClient; this.ruleIndexer = ruleIndexer; this.activeRuleIndexer = activeRuleIndexer; @@ -130,6 +130,8 @@ public class RegisterRules implements Startable { keysToIndex.addAll(removedRules.stream().map(RuleDefinitionDto::getKey).collect(Collectors.toList())); persistRepositories(dbSession, context.repositories()); + // FIXME lack of resiliency, active rules index is corrupted if rule index fails + // to be updated. Only a single DB commit should be executed. ruleIndexer.commitAndIndex(dbSession, keysToIndex); activeRuleIndexer.commitAndIndex(dbSession, changes); profiler.stopDebug(); @@ -506,7 +508,7 @@ public class RegisterRules implements Startable { * The side effect of this approach is that extended repositories will not be managed the same way. * If an extended repository do not exists anymore, then related active rules will be removed. */ - private List removeActiveRulesOnStillExistingRepositories(DbSession session, Collection removedRules, RulesDefinition.Context context) { + private List removeActiveRulesOnStillExistingRepositories(DbSession dbSession, Collection removedRules, RulesDefinition.Context context) { List repositoryKeys = newArrayList(Iterables.transform(context.repositories(), RulesDefinition.Repository::key)); List changes = new ArrayList<>(); @@ -515,7 +517,7 @@ public class RegisterRules implements Startable { // SONAR-4642 Remove active rules only when repository still exists if (repositoryKeys.contains(rule.getRepositoryKey())) { profiler.start(); - changes.addAll(ruleActivator.delete(session, rule)); + changes.addAll(qProfileRules.deleteRule(dbSession, rule)); profiler.stopDebug(format("Remove active rule for rule %s", rule.getKey())); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java index fb8e06b7bcd..10c1c376534 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java @@ -28,7 +28,7 @@ import org.sonar.api.utils.System2; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.rule.index.RuleIndexer; import static com.google.common.base.Preconditions.checkArgument; @@ -40,14 +40,14 @@ public class DeleteAction implements RulesWsAction { private final System2 system2; private final RuleIndexer ruleIndexer; private final DbClient dbClient; - private final RuleActivator ruleActivator; + private final QProfileRules qProfileRules; private final RuleWsSupport ruleWsSupport; - public DeleteAction(System2 system2, RuleIndexer ruleIndexer, DbClient dbClient, RuleActivator ruleActivator, RuleWsSupport ruleWsSupport) { + public DeleteAction(System2 system2, RuleIndexer ruleIndexer, DbClient dbClient, QProfileRules qProfileRules, RuleWsSupport ruleWsSupport) { this.system2 = system2; this.ruleIndexer = ruleIndexer; this.dbClient = dbClient; - this.ruleActivator = ruleActivator; + this.qProfileRules = qProfileRules; this.ruleWsSupport = ruleWsSupport; } @@ -80,14 +80,12 @@ public class DeleteAction implements RulesWsAction { RuleDefinitionDto rule = dbClient.ruleDao().selectOrFailDefinitionByKey(dbSession, ruleKey); checkArgument(rule.isCustomRule(), "Rule '%s' cannot be deleted because it is not a custom rule", rule.getKey().toString()); - // For custom rule, first deactivate the rule on all profiles - if (rule.isCustomRule()) { - ruleActivator.delete(dbSession, rule); - } + qProfileRules.deleteRule(dbSession, rule); rule.setStatus(RuleStatus.REMOVED); rule.setUpdatedAt(system2.now()); dbClient.ruleDao().update(dbSession, rule); + ruleIndexer.commitAndIndex(dbSession, ruleKey); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java index 310a47a6a19..b966d46665a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java @@ -24,7 +24,6 @@ import java.util.Optional; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.profiles.RulesProfile; import org.sonar.api.rule.Severity; import org.sonar.api.rules.RulePriority; import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition; @@ -62,9 +61,8 @@ public class BuiltInQProfileUpdateImplTest { public UserSessionRule userSession = UserSessionRule.standalone(); private System2 system2 = new TestSystem2().setNow(NOW); private ActiveRuleIndexer activeRuleIndexer = mock(ActiveRuleIndexer.class); - private RuleActivatorContextFactory contextFactory = new RuleActivatorContextFactory(db.getDbClient()); private TypeValidations typeValidations = new TypeValidations(asList(new StringTypeValidation(), new IntegerTypeValidation())); - private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), null, contextFactory, typeValidations, activeRuleIndexer, userSession); + private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), typeValidations, userSession); private BuiltInQProfileUpdateImpl underTest = new BuiltInQProfileUpdateImpl(db.getDbClient(), ruleActivator, activeRuleIndexer); @@ -189,6 +187,48 @@ public class BuiltInQProfileUpdateImplTest { assertThatProfileIsMarkedAsUpdated(persistedProfile); } +// @Test +// public void propagate_new_activation_to_profiles() { +// RuleDefinitionDto rule = createJavaRule(); +// QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), +// p -> p.setLanguage(rule.getLanguage()) +// .setIsBuiltIn(true)); +// QProfileDto childProfile = createChildProfile(profile); +// QProfileDto grandchildProfile = createChildProfile(childProfile); +// +// List changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), +// RulesProfileDto.from(profile)); +// +// assertThat(changes).hasSize(3); +// assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); +// assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); +// assertThatRuleIsActivated(grandchildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); +// } +// +// @Test +// public void deactivateOnBuiltInProfile_activate_rule_on_child_profiles() { +// RuleDefinitionDto rule = createJavaRule(); +// QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), +// p -> p.setLanguage(rule.getLanguage()) +// .setIsBuiltIn(true)); +// QProfileDto childProfile = createChildProfile(profile); +// QProfileDto grandchildProfile = createChildProfile(childProfile); +// +// List changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), +// RulesProfileDto.from(profile)); +// +// assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); +// assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); +// assertThatRuleIsActivated(grandchildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); +// +// changes = underTest.deactivateOnBuiltInRulesProfile(db.getSession(), RulesProfileDto.from(profile), rule.getKey(), false); +// +// assertThat(changes).hasSize(3); +// assertThatRuleIsNotPresent(profile, rule); +// assertThatRuleIsNotPresent(childProfile, rule); +// assertThatRuleIsNotPresent(grandchildProfile, rule); +// } + private static void assertThatRuleIsNewlyActivated(List activeRules, RuleDefinitionDto rule, RulePriority severity) { ActiveRuleDto activeRule = findRule(activeRules, rule).get(); @@ -244,10 +284,6 @@ public class BuiltInQProfileUpdateImplTest { .findFirst(); } - private static void activateRuleInDef(RulesProfile apiProfile, RuleDefinitionDto rule, RulePriority severity) { - apiProfile.activateRule(org.sonar.api.rules.Rule.create(rule.getRepositoryKey(), rule.getRuleKey()), severity); - } - private void activateRuleInDb(RulesProfileDto profile, RuleDefinitionDto rule, RulePriority severity) { ActiveRuleDto dto = new ActiveRuleDto() .setProfileId(profile.getId()) diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonTest.java index 5eb6cec3d15..dbcb29e488b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonTest.java @@ -26,6 +26,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.sonar.api.config.internal.MapSettings; import org.sonar.api.rule.Severity; import org.sonar.api.server.rule.RuleParamType; import org.sonar.api.utils.System2; @@ -41,10 +42,12 @@ import org.sonar.server.qualityprofile.QProfileComparison.ActiveRuleDiff; import org.sonar.server.qualityprofile.QProfileComparison.QProfileComparisonResult; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndex; +import org.sonar.server.rule.index.RuleIndexDefinition; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.util.IntegerTypeValidation; import org.sonar.server.util.TypeValidations; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; @@ -55,11 +58,11 @@ public class QProfileComparisonTest { @Rule public DbTester dbTester = DbTester.create(); @Rule - public EsTester esTester = new EsTester(); + public EsTester esTester = new EsTester(RuleIndexDefinition.createForTest(new MapSettings().asConfig())); private DbClient db; private DbSession dbSession; - private RuleActivator ruleActivator; + private QProfileRules qProfileRules; private QProfileComparison comparison; private RuleDto xooRule1; @@ -71,15 +74,10 @@ public class QProfileComparisonTest { public void before() { db = dbTester.getDbClient(); dbSession = db.openSession(false); - ruleActivator = new RuleActivator( - System2.INSTANCE, - db, - new RuleIndex(esTester.client(), System2.INSTANCE), - new RuleActivatorContextFactory(db), - new TypeValidations(singletonList(new IntegerTypeValidation())), - new ActiveRuleIndexer(db, esTester.client()), - userSession - ); + RuleIndex ruleIndex = new RuleIndex(esTester.client(), System2.INSTANCE); + ActiveRuleIndexer activeRuleIndexer = new ActiveRuleIndexer(db, esTester.client()); + RuleActivator ruleActivator = new RuleActivator(System2.INSTANCE, db, new TypeValidations(singletonList(new IntegerTypeValidation())), userSession); + qProfileRules = new QProfileRulesImpl(db, ruleActivator, ruleIndex, activeRuleIndexer); comparison = new QProfileComparison(db); xooRule1 = RuleTesting.newXooX1().setSeverity("MINOR"); @@ -119,9 +117,8 @@ public class QProfileComparisonTest { public void compare_same() { RuleActivation commonActivation = RuleActivation.create(xooRule1.getKey(), Severity.CRITICAL, ImmutableMap.of("min", "7", "max", "42")); - ruleActivator.activate(dbSession, commonActivation, left); - ruleActivator.activate(dbSession, commonActivation, right); - dbSession.commit(); + qProfileRules.activateAndCommit(dbSession, left, singleton(commonActivation)); + qProfileRules.activateAndCommit(dbSession, right, singleton(commonActivation)); QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); @@ -136,8 +133,7 @@ public class QProfileComparisonTest { @Test public void compare_only_left() { RuleActivation activation = RuleActivation.create(xooRule1.getKey()); - ruleActivator.activate(dbSession, activation, left); - dbSession.commit(); + qProfileRules.activateAndCommit(dbSession, left, singleton(activation)); QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); @@ -151,8 +147,7 @@ public class QProfileComparisonTest { @Test public void compare_only_right() { - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey()), right); - dbSession.commit(); + qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getKey()))); QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); @@ -166,9 +161,8 @@ public class QProfileComparisonTest { @Test public void compare_disjoint() { - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey()), left); - ruleActivator.activate(dbSession, RuleActivation.create(xooRule2.getKey()), right); - dbSession.commit(); + qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getKey()))); + qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule2.getKey()))); QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); @@ -182,9 +176,8 @@ public class QProfileComparisonTest { @Test public void compare_modified_severity() { - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), Severity.CRITICAL, null), left); - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), Severity.BLOCKER, null), right); - dbSession.commit(); + qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getKey(), Severity.CRITICAL, null))); + qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getKey(), Severity.BLOCKER, null))); QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); @@ -203,9 +196,8 @@ public class QProfileComparisonTest { @Test public void compare_modified_param() { - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "20")), left); - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "30")), right); - dbSession.commit(); + qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "20")))); + qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "30")))); QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); @@ -227,9 +219,8 @@ public class QProfileComparisonTest { @Test public void compare_different_params() { - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "20")), left); - ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("min", "5")), right); - dbSession.commit(); + qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "20")))); + qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("min", "5")))); QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileExportersTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileExportersTest.java index fa6a9a0c598..83059192909 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileExportersTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileExportersTest.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.Reader; import java.io.StringWriter; import java.io.Writer; +import java.util.Collection; import org.junit.Before; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -40,7 +41,6 @@ import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.db.rule.RuleParamDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.organization.DefaultOrganizationProvider; @@ -70,19 +70,17 @@ public class QProfileExportersTest { public DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private RuleFinder ruleFinder = new DefaultRuleFinder(db.getDbClient(), defaultOrganizationProvider); - private RuleActivator ruleActivator = mock(RuleActivator.class); private ProfileExporter[] exporters = new ProfileExporter[] { new StandardExporter(), new XooExporter()}; private ProfileImporter[] importers = new ProfileImporter[] { new XooProfileImporter(), new XooProfileImporterWithMessages(), new XooProfileImporterWithError()}; private RuleDefinitionDto rule; - private RuleParamDto ruleParam; - private QProfileExporters underTest = new QProfileExporters(db.getDbClient(), ruleFinder, ruleActivator, exporters, importers); + private QProfileRules qProfileRules = mock(QProfileRules.class); + private QProfileExporters underTest = new QProfileExporters(db.getDbClient(), ruleFinder, qProfileRules, exporters, importers); @Before public void setUp() { rule = db.rules().insert(r -> r.setLanguage("xoo").setRepositoryKey("SonarXoo").setRuleKey("R1")); - ruleParam = db.rules().insertRuleParam(rule); } @Test @@ -108,12 +106,16 @@ public class QProfileExportersTest { underTest.importXml(profile, "XooProfileImporter", toInputStream("", UTF_8), db.getSession()); ArgumentCaptor profileCapture = ArgumentCaptor.forClass(QProfileDto.class); - ArgumentCaptor activationCapture = ArgumentCaptor.forClass(RuleActivation.class); - verify(ruleActivator).activate(any(DbSession.class), activationCapture.capture(), profileCapture.capture()); + Class> collectionClass = (Class>) (Class) Collection.class; + ArgumentCaptor> activationCapture = ArgumentCaptor.forClass(collectionClass); + verify(qProfileRules).activateAndCommit(any(DbSession.class), profileCapture.capture(), activationCapture.capture()); assertThat(profileCapture.getValue().getKee()).isEqualTo(profile.getKee()); - assertThat(activationCapture.getValue().getRuleKey()).isEqualTo(rule.getKey()); - assertThat(activationCapture.getValue().getSeverity()).isEqualTo("CRITICAL"); + Collection activations = activationCapture.getValue(); + assertThat(activations).hasSize(1); + RuleActivation activation = activations.iterator().next(); + assertThat(activation.getRuleKey()).isEqualTo(rule.getKey()); + assertThat(activation.getSeverity()).isEqualTo("CRITICAL"); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileResetImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileResetImplTest.java index 306f871d6ca..130fecf0201 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileResetImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileResetImplTest.java @@ -37,6 +37,7 @@ import org.sonar.server.util.StringTypeValidation; import org.sonar.server.util.TypeValidations; import static java.util.Arrays.asList; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; @@ -56,16 +57,17 @@ public class QProfileResetImplTest { private System2 system2 = new AlwaysIncreasingSystem2(); private ActiveRuleIndexer activeRuleIndexer = mock(ActiveRuleIndexer.class); - private RuleActivatorContextFactory contextFactory = new RuleActivatorContextFactory(db.getDbClient()); private TypeValidations typeValidations = new TypeValidations(asList(new StringTypeValidation(), new IntegerTypeValidation())); - private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), null, contextFactory, typeValidations, activeRuleIndexer, userSession); + private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), typeValidations, userSession); + private QProfileTree qProfileTree = new QProfileTreeImpl(db.getDbClient(), ruleActivator, system2, activeRuleIndexer); + private QProfileRules qProfileRules = new QProfileRulesImpl(db.getDbClient(), ruleActivator, null, activeRuleIndexer); private QProfileResetImpl underTest = new QProfileResetImpl(db.getDbClient(), ruleActivator, activeRuleIndexer); @Test public void reset() { QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(LANGUAGE)); RuleDefinitionDto existingRule = db.rules().insert(r -> r.setLanguage(LANGUAGE)); - ruleActivator.activate(db.getSession(), RuleActivation.create(existingRule.getKey()), profile); + qProfileRules.activateAndCommit(db.getSession(), profile, singleton(RuleActivation.create(existingRule.getKey()))); RuleDefinitionDto newRule = db.rules().insert(r -> r.setLanguage(LANGUAGE)); BulkChangeResult result = underTest.reset(db.getSession(), profile, singletonList(RuleActivation.create(newRule.getKey()))); @@ -83,10 +85,10 @@ public class QProfileResetImplTest { public void inherited_rules_are_not_disabled() { QProfileDto parentProfile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(LANGUAGE)); QProfileDto childProfile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(LANGUAGE)); - ruleActivator.setParentAndCommit(db.getSession(), childProfile, parentProfile); + qProfileTree.setParentAndCommit(db.getSession(), childProfile, parentProfile); RuleDefinitionDto existingRule = db.rules().insert(r -> r.setLanguage(LANGUAGE)); - ruleActivator.activate(db.getSession(), RuleActivation.create(existingRule.getKey()), parentProfile); - ruleActivator.activate(db.getSession(), RuleActivation.create(existingRule.getKey()), childProfile); + qProfileRules.activateAndCommit(db.getSession(), parentProfile, singleton(RuleActivation.create(existingRule.getKey()))); + qProfileRules.activateAndCommit(db.getSession(), childProfile, singleton(RuleActivation.create(existingRule.getKey()))); RuleDefinitionDto newRule = db.rules().insert(r -> r.setLanguage(LANGUAGE)); underTest.reset(db.getSession(), childProfile, singletonList(RuleActivation.create(newRule.getKey()))); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleImplTest.java similarity index 80% rename from server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java rename to server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleImplTest.java index 5e3622e0e2a..cd562543f93 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleImplTest.java @@ -40,7 +40,6 @@ import org.sonar.db.DbTester; import org.sonar.db.qualityprofile.ActiveRuleParamDto; import org.sonar.db.qualityprofile.OrgActiveRuleDto; import org.sonar.db.qualityprofile.QProfileDto; -import org.sonar.db.qualityprofile.RulesProfileDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleParamDto; @@ -60,6 +59,7 @@ import org.sonar.server.util.TypeValidations; import static com.google.common.collect.ImmutableMap.of; import static java.util.Arrays.asList; import static java.util.Collections.emptyMap; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; @@ -71,7 +71,7 @@ import static org.sonar.api.rule.Severity.MINOR; import static org.sonar.db.rule.RuleTesting.newCustomRule; import static org.sonar.server.qualityprofile.ActiveRule.Inheritance.INHERITED; -public class RuleActivatorTest { +public class QProfileRuleImplTest { @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -84,13 +84,12 @@ public class RuleActivatorTest { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); private RuleIndex ruleIndex = new RuleIndex(es.client(), system2); - private RuleActivatorContextFactory contextFactory = new RuleActivatorContextFactory(db.getDbClient()); private ActiveRuleIndexer activeRuleIndexer = new ActiveRuleIndexer(db.getDbClient(), es.client()); private RuleIndexer ruleIndexer = new RuleIndexer(es.client(), db.getDbClient()); private TypeValidations typeValidations = new TypeValidations(asList(new StringTypeValidation(), new IntegerTypeValidation())); - private RuleActivator underTest = new RuleActivator(system2, db.getDbClient(), ruleIndex, contextFactory, typeValidations, activeRuleIndexer, - userSession); + private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), typeValidations, userSession); + private QProfileRules underTest = new QProfileRulesImpl(db.getDbClient(), ruleActivator, ruleIndex, activeRuleIndexer); @Test public void system_activates_rule_without_parameters() { @@ -343,7 +342,7 @@ public class RuleActivatorTest { QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage("js")); RuleActivation activation = RuleActivation.create(rule.getKey()); - expectFailure("Rule " + rule.getKey() + " and profile " + profile.getKee() + " have different languages", () -> activate(profile, activation)); + expectFailure("java rule " + rule.getKey() + " cannot be activated on js profile " + profile.getKee(), () -> activate(profile, activation)); } @Test @@ -444,7 +443,7 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); RuleKey ruleKey = RuleKey.parse("unknown:xxx"); - expectFailure("Rule not found: " + ruleKey, () -> underTest.deactivate(db.getSession(), profile, ruleKey)); + expectFailure("Rule not found: " + ruleKey, () -> underTest.deactivateAndCommit(db.getSession(), profile, singleton(ruleKey))); } @Test @@ -678,37 +677,37 @@ public class RuleActivatorTest { @Test public void reset_parent_is_not_propagated_when_child_overrides() { RuleDefinitionDto rule = createRule(); - QProfileDto parentProfile = createProfile(rule); - QProfileDto childProfile = createChildProfile(parentProfile); - QProfileDto grandchildProfile = createChildProfile(childProfile); + QProfileDto baseProfile = createProfile(rule); + QProfileDto childProfile = createChildProfile(baseProfile); + QProfileDto grandChildProfile = createChildProfile(childProfile); RuleActivation activation = RuleActivation.create(rule.getKey(), CRITICAL, null); - List changes = activate(parentProfile, activation); - assertThatRuleIsActivated(parentProfile, rule, changes, CRITICAL, null, emptyMap()); + List changes = activate(baseProfile, activation); + assertThatRuleIsActivated(baseProfile, rule, changes, CRITICAL, null, emptyMap()); assertThatRuleIsActivated(childProfile, rule, changes, CRITICAL, INHERITED, emptyMap()); - assertThatRuleIsActivated(grandchildProfile, rule, changes, CRITICAL, INHERITED, emptyMap()); + assertThatRuleIsActivated(grandChildProfile, rule, changes, CRITICAL, INHERITED, emptyMap()); assertThat(changes).hasSize(3); RuleActivation childActivation = RuleActivation.create(rule.getKey(), BLOCKER, null); changes = activate(childProfile, childActivation); assertThatRuleIsUpdated(childProfile, rule, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); - assertThatRuleIsUpdated(grandchildProfile, rule, BLOCKER, INHERITED, emptyMap()); + assertThatRuleIsUpdated(grandChildProfile, rule, BLOCKER, INHERITED, emptyMap()); assertThat(changes).hasSize(2); // Reset on parent do not change child nor grandchild RuleActivation resetActivation = RuleActivation.createReset(rule.getKey()); - changes = activate(parentProfile, resetActivation); - assertThatRuleIsUpdated(parentProfile, rule, rule.getSeverityString(), null, emptyMap()); + changes = activate(baseProfile, resetActivation); + assertThatRuleIsUpdated(baseProfile, rule, rule.getSeverityString(), null, emptyMap()); assertThatRuleIsUpdated(childProfile, rule, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); - assertThatRuleIsUpdated(grandchildProfile, rule, BLOCKER, INHERITED, emptyMap()); + assertThatRuleIsUpdated(grandChildProfile, rule, BLOCKER, INHERITED, emptyMap()); assertThat(changes).hasSize(1); // Reset on child change grandchild resetActivation = RuleActivation.createReset(rule.getKey()); changes = activate(childProfile, resetActivation); - assertThatRuleIsUpdated(parentProfile, rule, rule.getSeverityString(), null, emptyMap()); + assertThatRuleIsUpdated(baseProfile, rule, rule.getSeverityString(), null, emptyMap()); assertThatRuleIsUpdated(childProfile, rule, rule.getSeverityString(), INHERITED, emptyMap()); - assertThatRuleIsUpdated(grandchildProfile, rule, rule.getSeverityString(), INHERITED, emptyMap()); + assertThatRuleIsUpdated(grandChildProfile, rule, rule.getSeverityString(), INHERITED, emptyMap()); assertThat(changes).hasSize(2); } @@ -724,119 +723,6 @@ public class RuleActivatorTest { assertThat(changes).hasSize(0); } - @Test - public void unset_parent_when_no_parent_does_not_fail() { - RuleDefinitionDto rule = createRule(); - QProfileDto profile = createProfile(rule); - underTest.setParentAndCommit(db.getSession(), profile, null); - } - - @Test - public void set_itself_as_parent_fails() { - RuleDefinitionDto rule = createRule(); - QProfileDto profile = createProfile(rule); - - expectedException.expect(BadRequestException.class); - expectedException.expectMessage(" can not be selected as parent of "); - underTest.setParentAndCommit(db.getSession(), profile, profile); - } - - @Test - public void set_child_as_parent_fails() { - RuleDefinitionDto rule = createRule(); - QProfileDto parentProfile = createProfile(rule); - QProfileDto childProfile = createChildProfile(parentProfile); - - expectedException.expect(BadRequestException.class); - expectedException.expectMessage(" can not be selected as parent of "); - underTest.setParentAndCommit(db.getSession(), parentProfile, childProfile); - } - - @Test - public void set_grandchild_as_parent_fails() { - RuleDefinitionDto rule = createRule(); - QProfileDto parentProfile = createProfile(rule); - QProfileDto childProfile = createChildProfile(parentProfile); - QProfileDto grandchildProfile = createChildProfile(childProfile); - - expectedException.expect(BadRequestException.class); - expectedException.expectMessage(" can not be selected as parent of "); - underTest.setParentAndCommit(db.getSession(), parentProfile, grandchildProfile); - } - - @Test - public void cannot_set_parent_if_language_is_different() { - RuleDefinitionDto rule1 = db.rules().insert(r -> r.setLanguage("foo")); - RuleDefinitionDto rule2 = db.rules().insert(r -> r.setLanguage("bar")); - - QProfileDto parentProfile = createProfile(rule1); - List changes = activate(parentProfile, RuleActivation.create(rule1.getKey())); - assertThat(changes).hasSize(1); - - QProfileDto childProfile = createProfile(rule2); - changes = activate(childProfile, RuleActivation.create(rule2.getKey())); - assertThat(changes).hasSize(1); - - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Cannot set the profile"); - - underTest.setParentAndCommit(db.getSession(), childProfile, parentProfile); - } - - @Test - public void set_then_unset_parent() { - RuleDefinitionDto rule1 = createJavaRule(); - RuleDefinitionDto rule2 = createJavaRule(); - - QProfileDto profile1 = createProfile(rule1); - List changes = activate(profile1, RuleActivation.create(rule1.getKey())); - assertThat(changes).hasSize(1); - - QProfileDto profile2 = createProfile(rule2); - changes = activate(profile2, RuleActivation.create(rule2.getKey())); - assertThat(changes).hasSize(1); - - changes = underTest.setParentAndCommit(db.getSession(), profile2, profile1); - assertThat(changes).hasSize(1); - assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), INHERITED, emptyMap()); - assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); - - changes = underTest.setParentAndCommit(db.getSession(), profile2, null); - assertThat(changes).hasSize(1); - assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); - assertThatRuleIsNotPresent(profile2, rule1); - } - - @Test - public void set_then_unset_parent_keep_overridden_rules() { - RuleDefinitionDto rule1 = createJavaRule(); - RuleDefinitionDto rule2 = createJavaRule(); - QProfileDto profile1 = createProfile(rule1); - List changes = activate(profile1, RuleActivation.create(rule1.getKey())); - assertThat(changes).hasSize(1); - - QProfileDto profile2 = createProfile(rule2); - changes = activate(profile2, RuleActivation.create(rule2.getKey())); - assertThat(changes).hasSize(1); - - changes = underTest.setParentAndCommit(db.getSession(), profile2, profile1); - assertThat(changes).hasSize(1); - assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), INHERITED, emptyMap()); - assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); - - RuleActivation activation = RuleActivation.create(rule1.getKey(), BLOCKER, null); - changes = activate(profile2, activation); - assertThat(changes).hasSize(1); - assertThatRuleIsUpdated(profile2, rule1, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); - assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); - - changes = underTest.setParentAndCommit(db.getSession(), profile2, null); - assertThat(changes).hasSize(1); - // Not testing changes here since severity is not set in changelog - assertThatRuleIsActivated(profile2, rule1, null, BLOCKER, null, emptyMap()); - assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); - } - @Test public void bulk_activation() { int bulkSize = SearchOptions.MAX_LIMIT + 10 + new Random().nextInt(100); @@ -854,7 +740,7 @@ public class RuleActivatorTest { RuleQuery ruleQuery = new RuleQuery() .setRepositories(singletonList(repositoryKey)); - BulkChangeResult bulkChangeResult = underTest.bulkActivateAndCommit(db.getSession(), ruleQuery, profile, MINOR); + BulkChangeResult bulkChangeResult = underTest.bulkActivateAndCommit(db.getSession(), profile, ruleQuery, MINOR); assertThat(bulkChangeResult.countFailed()).isEqualTo(0); assertThat(bulkChangeResult.countSucceeded()).isEqualTo(bulkSize); @@ -881,7 +767,7 @@ public class RuleActivatorTest { RuleQuery ruleQuery = new RuleQuery() .setRepositories(singletonList(repositoryKey)); - BulkChangeResult bulkChangeResult = underTest.bulkActivateAndCommit(db.getSession(), ruleQuery, profile, MINOR); + BulkChangeResult bulkChangeResult = underTest.bulkActivateAndCommit(db.getSession(), profile, ruleQuery, MINOR); assertThat(bulkChangeResult.countFailed()).isEqualTo(0); assertThat(bulkChangeResult.countSucceeded()).isEqualTo(bulkSize); @@ -889,7 +775,7 @@ public class RuleActivatorTest { assertThat(db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile)).hasSize(bulkSize); // Now deactivate all rules - bulkChangeResult = underTest.bulkDeactivateAndCommit(db.getSession(), ruleQuery, profile); + bulkChangeResult = underTest.bulkDeactivateAndCommit(db.getSession(), profile, ruleQuery); assertThat(bulkChangeResult.countFailed()).isEqualTo(0); assertThat(bulkChangeResult.countSucceeded()).isEqualTo(bulkSize); @@ -913,7 +799,7 @@ public class RuleActivatorTest { RuleQuery ruleQuery = new RuleQuery() .setQProfile(childProfile); - BulkChangeResult bulkChangeResult = underTest.bulkDeactivateAndCommit(db.getSession(), ruleQuery, childProfile); + BulkChangeResult bulkChangeResult = underTest.bulkDeactivateAndCommit(db.getSession(), childProfile, ruleQuery); assertThat(bulkChangeResult.countFailed()).isEqualTo(1); assertThat(bulkChangeResult.countSucceeded()).isEqualTo(0); @@ -938,7 +824,7 @@ public class RuleActivatorTest { RuleQuery query = new RuleQuery() .setRuleKey(rule1.getRuleKey()) .setQProfile(parentProfile); - BulkChangeResult result = underTest.bulkActivateAndCommit(db.getSession(), query, parentProfile, "BLOCKER"); + BulkChangeResult result = underTest.bulkActivateAndCommit(db.getSession(), parentProfile, query, "BLOCKER"); assertThat(result.getChanges()).hasSize(3); assertThat(result.countSucceeded()).isEqualTo(1); @@ -955,69 +841,6 @@ public class RuleActivatorTest { assertThatRuleIsActivated(grandchildProfile, rule2, null, rule2.getSeverityString(), INHERITED, emptyMap()); } - @Test - public void activateOnBuiltInProfile_throws_IAE_when_profile_is_not_built_in() { - RuleDefinitionDto rule = createJavaRule(); - QProfileDto profile = createProfile(rule); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Rules profile must be a built-in profile: " + profile.getRulesProfileUuid()); - - underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); - } - - @Test - public void activateOnBuiltInProfile_activate_rule_on_child_profiles() { - RuleDefinitionDto rule = createJavaRule(); - QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), - p -> p.setLanguage(rule.getLanguage()) - .setIsBuiltIn(true)); - QProfileDto childProfile = createChildProfile(profile); - QProfileDto grandchildProfile = createChildProfile(childProfile); - - List changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); - - assertThat(changes).hasSize(3); - assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); - assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); - assertThatRuleIsActivated(grandchildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); - } - - @Test - public void deactivateOnBuiltInProfile_throws_IAE_when_profile_is_not_built_in() { - RuleDefinitionDto rule = createJavaRule(); - QProfileDto profile = createProfile(rule); - activate(profile, RuleActivation.create(rule.getKey())); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Rules profile must be a built-in profile: " + profile.getRulesProfileUuid()); - - underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); - } - - @Test - public void deactivateOnBuiltInProfile_activate_rule_on_child_profiles() { - RuleDefinitionDto rule = createJavaRule(); - QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), - p -> p.setLanguage(rule.getLanguage()) - .setIsBuiltIn(true)); - QProfileDto childProfile = createChildProfile(profile); - QProfileDto grandchildProfile = createChildProfile(childProfile); - - List changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); - - assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); - assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); - assertThatRuleIsActivated(grandchildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); - - changes = underTest.deactivateOnBuiltInRulesProfile(db.getSession(), RulesProfileDto.from(profile), rule.getKey(), false); - - assertThat(changes).hasSize(3); - assertThatRuleIsNotPresent(profile, rule); - assertThatRuleIsNotPresent(childProfile, rule); - assertThatRuleIsNotPresent(grandchildProfile, rule); - } - @Test public void delete_rule_from_all_profiles() { RuleDefinitionDto rule = createRule(); @@ -1032,7 +855,7 @@ public class RuleActivatorTest { activate(grandChildProfile, overrideActivation); // Reset on parent do not change child nor grandchild - List changes = underTest.delete(db.getSession(), rule); + List changes = underTest.deleteRule(db.getSession(), rule); assertThatRuleIsNotPresent(parentProfile, rule); assertThatRuleIsNotPresent(childProfile, rule); @@ -1044,21 +867,14 @@ public class RuleActivatorTest { } @Test - public void activation_errors_are_ignored_when_setting_a_parent() { - RuleDefinitionDto rule1 = createJavaRule(); - RuleDefinitionDto rule2 = createJavaRule(); - QProfileDto parentProfile = createProfile(rule1); - activate(parentProfile, RuleActivation.create(rule1.getKey())); - activate(parentProfile, RuleActivation.create(rule2.getKey())); - - rule1.setStatus(RuleStatus.REMOVED); - db.rules().update(rule1); + public void activation_fails_when_profile_is_built_in() { + RuleDefinitionDto rule = createRule(); + QProfileDto builtInProfile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(rule.getLanguage()).setIsBuiltIn(true)); - QProfileDto childProfile = createProfile(rule1); - List changes = underTest.setParentAndCommit(db.getSession(), childProfile, parentProfile); + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("The built-in profile " + builtInProfile.getName() + " is read-only and can't be updated"); - assertThatRuleIsNotPresent(childProfile, rule1); - assertThatRuleIsActivated(childProfile, rule2, changes, rule2.getSeverityString(), INHERITED, emptyMap()); + underTest.activateAndCommit(db.getSession(), builtInProfile, singleton(RuleActivation.create(rule.getKey()))); } private void assertThatProfileHasNoActiveRules(QProfileDto profile) { @@ -1067,11 +883,11 @@ public class RuleActivatorTest { } private List deactivate(QProfileDto profile, RuleDefinitionDto rule) { - return underTest.deactivate(db.getSession(), profile, rule.getKey()); + return underTest.deactivateAndCommit(db.getSession(), profile, singleton(rule.getKey())); } private List activate(QProfileDto profile, RuleActivation activation) { - return underTest.activate(db.getSession(), activation, profile); + return underTest.activateAndCommit(db.getSession(), profile, singleton(activation)); } private QProfileDto createProfile(RuleDefinitionDto rule) { @@ -1079,7 +895,10 @@ public class RuleActivatorTest { } private QProfileDto createChildProfile(QProfileDto parent) { - return db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(parent.getLanguage()).setParentKee(parent.getKee())); + return db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p + .setLanguage(parent.getLanguage()) + .setParentKee(parent.getKee()) + .setName("Child of " + parent.getName())); } private void assertThatProfileIsUpdatedByUser(QProfileDto profile) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTesting.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTesting.java index d624e282a52..c64d7060877 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTesting.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTesting.java @@ -23,7 +23,8 @@ import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; /** - * Utility class for tests involving quality profiles + * Utility class for tests involving quality profiles. + * @deprecated replaced by {@link org.sonar.db.qualityprofile.QualityProfileDbTester} */ @Deprecated public class QProfileTesting { diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java new file mode 100644 index 00000000000..0012109e938 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java @@ -0,0 +1,272 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 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.server.qualityprofile; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import javax.annotation.Nullable; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.config.internal.MapSettings; +import org.sonar.api.rule.RuleStatus; +import org.sonar.api.rule.Severity; +import org.sonar.api.utils.System2; +import org.sonar.api.utils.internal.AlwaysIncreasingSystem2; +import org.sonar.db.DbTester; +import org.sonar.db.qualityprofile.ActiveRuleParamDto; +import org.sonar.db.qualityprofile.OrgActiveRuleDto; +import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.server.es.EsTester; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; +import org.sonar.server.rule.index.RuleIndexDefinition; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.util.IntegerTypeValidation; +import org.sonar.server.util.StringTypeValidation; +import org.sonar.server.util.TypeValidations; + +import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singleton; +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.api.rule.Severity.BLOCKER; +import static org.sonar.server.qualityprofile.ActiveRule.Inheritance.INHERITED; + +public class QProfileTreeImplTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private System2 system2 = new AlwaysIncreasingSystem2(); + @Rule + public DbTester db = DbTester.create(system2); + @Rule + public EsTester es = new EsTester(RuleIndexDefinition.createForTest(new MapSettings().asConfig())); + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + private ActiveRuleIndexer activeRuleIndexer = new ActiveRuleIndexer(db.getDbClient(), es.client()); + private TypeValidations typeValidations = new TypeValidations(asList(new StringTypeValidation(), new IntegerTypeValidation())); + private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), typeValidations, userSession); + private QProfileRules qProfileRules = new QProfileRulesImpl(db.getDbClient(), ruleActivator, null, activeRuleIndexer); + private QProfileTree underTest = new QProfileTreeImpl(db.getDbClient(), ruleActivator, System2.INSTANCE, activeRuleIndexer); + + @Test + public void set_itself_as_parent_fails() { + RuleDefinitionDto rule = createRule(); + QProfileDto profile = createProfile(rule); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(" can not be selected as parent of "); + + underTest.setParentAndCommit(db.getSession(), profile, profile); + } + + @Test + public void set_child_as_parent_fails() { + RuleDefinitionDto rule = createRule(); + QProfileDto parentProfile = createProfile(rule); + QProfileDto childProfile = createChildProfile(parentProfile); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(" can not be selected as parent of "); + underTest.setParentAndCommit(db.getSession(), parentProfile, childProfile); + } + + @Test + public void set_grandchild_as_parent_fails() { + RuleDefinitionDto rule = createRule(); + QProfileDto parentProfile = createProfile(rule); + QProfileDto childProfile = createChildProfile(parentProfile); + QProfileDto grandchildProfile = createChildProfile(childProfile); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(" can not be selected as parent of "); + underTest.setParentAndCommit(db.getSession(), parentProfile, grandchildProfile); + } + + @Test + public void cannot_set_parent_if_language_is_different() { + RuleDefinitionDto rule1 = db.rules().insert(r -> r.setLanguage("foo")); + RuleDefinitionDto rule2 = db.rules().insert(r -> r.setLanguage("bar")); + + QProfileDto parentProfile = createProfile(rule1); + List changes = activate(parentProfile, RuleActivation.create(rule1.getKey())); + assertThat(changes).hasSize(1); + + QProfileDto childProfile = createProfile(rule2); + changes = activate(childProfile, RuleActivation.create(rule2.getKey())); + assertThat(changes).hasSize(1); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Cannot set the profile"); + + underTest.setParentAndCommit(db.getSession(), childProfile, parentProfile); + } + + @Test + public void set_then_unset_parent() { + RuleDefinitionDto rule1 = createJavaRule(); + RuleDefinitionDto rule2 = createJavaRule(); + + QProfileDto profile1 = createProfile(rule1); + List changes = activate(profile1, RuleActivation.create(rule1.getKey())); + assertThat(changes).hasSize(1); + + QProfileDto profile2 = createProfile(rule2); + changes = activate(profile2, RuleActivation.create(rule2.getKey())); + assertThat(changes).hasSize(1); + + changes = underTest.setParentAndCommit(db.getSession(), profile2, profile1); + assertThat(changes).hasSize(1); + assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), INHERITED, emptyMap()); + assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); + + changes = underTest.removeParentAndCommit(db.getSession(), profile2); + assertThat(changes).hasSize(1); + assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); + assertThatRuleIsNotPresent(profile2, rule1); + } + + @Test + public void set_then_unset_parent_keep_overridden_rules() { + RuleDefinitionDto rule1 = createJavaRule(); + RuleDefinitionDto rule2 = createJavaRule(); + QProfileDto profile1 = createProfile(rule1); + List changes = activate(profile1, RuleActivation.create(rule1.getKey())); + assertThat(changes).hasSize(1); + + QProfileDto profile2 = createProfile(rule2); + changes = activate(profile2, RuleActivation.create(rule2.getKey())); + assertThat(changes).hasSize(1); + + changes = underTest.setParentAndCommit(db.getSession(), profile2, profile1); + assertThat(changes).hasSize(1); + assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), INHERITED, emptyMap()); + assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); + + RuleActivation activation = RuleActivation.create(rule1.getKey(), BLOCKER, null); + changes = activate(profile2, activation); + assertThat(changes).hasSize(1); + assertThatRuleIsUpdated(profile2, rule1, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); + assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); + + changes = underTest.removeParentAndCommit(db.getSession(), profile2); + assertThat(changes).hasSize(1); + // Not testing changes here since severity is not set in changelog + assertThatRuleIsActivated(profile2, rule1, null, BLOCKER, null, emptyMap()); + assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); + } + + @Test + public void activation_errors_are_ignored_when_setting_a_parent() { + RuleDefinitionDto rule1 = createJavaRule(); + RuleDefinitionDto rule2 = createJavaRule(); + QProfileDto parentProfile = createProfile(rule1); + activate(parentProfile, RuleActivation.create(rule1.getKey())); + activate(parentProfile, RuleActivation.create(rule2.getKey())); + + rule1.setStatus(RuleStatus.REMOVED); + db.rules().update(rule1); + + QProfileDto childProfile = createProfile(rule1); + List changes = underTest.setParentAndCommit(db.getSession(), childProfile, parentProfile); + + assertThatRuleIsNotPresent(childProfile, rule1); + assertThatRuleIsActivated(childProfile, rule2, changes, rule2.getSeverityString(), INHERITED, emptyMap()); + } + + private List activate(QProfileDto profile, RuleActivation activation) { + return qProfileRules.activateAndCommit(db.getSession(), profile, singleton(activation)); + } + + private QProfileDto createProfile(RuleDefinitionDto rule) { + return db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(rule.getLanguage())); + } + + private QProfileDto createChildProfile(QProfileDto parent) { + return db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p + .setLanguage(parent.getLanguage()) + .setParentKee(parent.getKee()) + .setName("Child of " + parent.getName())); + } + + private void assertThatRuleIsActivated(QProfileDto profile, RuleDefinitionDto rule, @Nullable List changes, + String expectedSeverity, @Nullable ActiveRule.Inheritance expectedInheritance, Map expectedParams) { + OrgActiveRuleDto activeRule = db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile) + .stream() + .filter(ar -> ar.getRuleKey().equals(rule.getKey())) + .findFirst() + .orElseThrow(IllegalStateException::new); + + assertThat(activeRule.getSeverityString()).isEqualTo(expectedSeverity); + assertThat(activeRule.getInheritance()).isEqualTo(expectedInheritance != null ? expectedInheritance.name() : null); + assertThat(activeRule.getCreatedAt()).isNotNull(); + assertThat(activeRule.getUpdatedAt()).isNotNull(); + + List params = db.getDbClient().activeRuleDao().selectParamsByActiveRuleId(db.getSession(), activeRule.getId()); + assertThat(params).hasSize(expectedParams.size()); + + if (changes != null) { + ActiveRuleChange change = changes.stream() + .filter(c -> c.getActiveRule().getId().equals(activeRule.getId())) + .findFirst().orElseThrow(IllegalStateException::new); + assertThat(change.getInheritance()).isEqualTo(expectedInheritance); + assertThat(change.getSeverity()).isEqualTo(expectedSeverity); + assertThat(change.getType()).isEqualTo(ActiveRuleChange.Type.ACTIVATED); + } + } + + private void assertThatRuleIsNotPresent(QProfileDto profile, RuleDefinitionDto rule) { + Optional activeRule = db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile) + .stream() + .filter(ar -> ar.getRuleKey().equals(rule.getKey())) + .findFirst(); + + assertThat(activeRule).isEmpty(); + } + + private void assertThatRuleIsUpdated(QProfileDto profile, RuleDefinitionDto rule, + String expectedSeverity, @Nullable ActiveRule.Inheritance expectedInheritance, Map expectedParams) { + OrgActiveRuleDto activeRule = db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile) + .stream() + .filter(ar -> ar.getRuleKey().equals(rule.getKey())) + .findFirst() + .orElseThrow(IllegalStateException::new); + + assertThat(activeRule.getSeverityString()).isEqualTo(expectedSeverity); + assertThat(activeRule.getInheritance()).isEqualTo(expectedInheritance != null ? expectedInheritance.name() : null); + assertThat(activeRule.getCreatedAt()).isNotNull(); + assertThat(activeRule.getUpdatedAt()).isNotNull(); + + List params = db.getDbClient().activeRuleDao().selectParamsByActiveRuleId(db.getSession(), activeRule.getId()); + assertThat(params).hasSize(expectedParams.size()); + } + + private RuleDefinitionDto createRule() { + return db.rules().insert(r -> r.setSeverity(Severity.MAJOR)); + } + + private RuleDefinitionDto createJavaRule() { + return db.rules().insert(r -> r.setSeverity(Severity.MAJOR).setLanguage("java")); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java index 637404fe1d5..3bbcaa35ced 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java @@ -47,6 +47,7 @@ import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.util.TypeValidations; +import static java.util.Collections.singleton; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.apache.commons.lang.math.RandomUtils.nextLong; import static org.assertj.core.api.Assertions.assertThat; @@ -78,22 +79,22 @@ public class RegisterQualityProfilesNotificationTest { public ExpectedException expectedException = ExpectedException.none(); @Rule public BuiltInQProfileRepositoryRule builtInQProfileRepositoryRule = new BuiltInQProfileRepositoryRule(); - @Rule public LogTester logTester = new LogTester(); + private DbClient dbClient = db.getDbClient(); private TypeValidations typeValidations = mock(TypeValidations.class); private ActiveRuleIndexer activeRuleIndexer = mock(ActiveRuleIndexer.class); private BuiltInQProfileInsert builtInQProfileInsert = new BuiltInQProfileInsertImpl(dbClient, system2, UuidFactoryFast.getInstance(), typeValidations, activeRuleIndexer); - private RuleActivator ruleActivator = new RuleActivator(system2, dbClient, mock(RuleIndex.class), new RuleActivatorContextFactory(dbClient), typeValidations, activeRuleIndexer, - userSessionRule); + private RuleActivator ruleActivator = new RuleActivator(system2, dbClient, typeValidations, userSessionRule); + private QProfileRules qProfileRules = new QProfileRulesImpl(dbClient, ruleActivator, mock(RuleIndex.class), activeRuleIndexer); private BuiltInQProfileUpdate builtInQProfileUpdate = new BuiltInQProfileUpdateImpl(dbClient, ruleActivator, activeRuleIndexer); private BuiltInQualityProfilesUpdateListener builtInQualityProfilesNotification = mock(BuiltInQualityProfilesUpdateListener.class); private RegisterQualityProfiles underTest = new RegisterQualityProfiles(builtInQProfileRepositoryRule, dbClient, builtInQProfileInsert, builtInQProfileUpdate, builtInQualityProfilesNotification, system2); @Test - public void does_not_send_notification_on_new_profile() { + public void do_not_send_notification_on_new_profile() { String language = newLanguageKey(); builtInQProfileRepositoryRule.add(newLanguage(language), "Sonar way"); builtInQProfileRepositoryRule.initialize(); @@ -104,7 +105,7 @@ public class RegisterQualityProfilesNotificationTest { } @Test - public void does_not_send_notification_when_built_in_profile_is_not_updated() { + public void do_not_send_notification_when_profile_is_not_updated() { String language = newLanguageKey(); RuleDefinitionDto dbRule = db.rules().insert(r -> r.setLanguage(language)); RulesProfileDto dbProfile = insertBuiltInProfile(language); @@ -118,7 +119,7 @@ public class RegisterQualityProfilesNotificationTest { } @Test - public void send_notification_when_built_in_profile_contains_new_rule() { + public void send_notification_when_a_new_rule_is_activated() { String language = newLanguageKey(); RuleDefinitionDto existingRule = db.rules().insert(r -> r.setLanguage(language)); RulesProfileDto dbProfile = insertBuiltInProfile(language); @@ -141,7 +142,7 @@ public class RegisterQualityProfilesNotificationTest { } @Test - public void send_notification_when_built_in_profile_contains_deactivated_rule() { + public void send_notification_when_a_rule_is_deactivated() { String language = newLanguageKey(); RuleDefinitionDto existingRule = db.rules().insert(r -> r.setLanguage(language)); RulesProfileDto dbProfile = insertBuiltInProfile(language); @@ -163,7 +164,7 @@ public class RegisterQualityProfilesNotificationTest { } @Test - public void only_send_one_notification_when_several_built_in_profiles_contain_new_rules() { + public void send_a_single_notification_when_multiple_rules_are_activated() { String language = newLanguageKey(); RuleDefinitionDto existingRule1 = db.rules().insert(r -> r.setLanguage(language)); @@ -197,7 +198,7 @@ public class RegisterQualityProfilesNotificationTest { } @Test - public void do_not_include_inherited_quality_profile_change_on_new_rule() { + public void notification_does_not_include_inherited_profiles_when_rule_is_added() { String language = newLanguageKey(); RuleDefinitionDto newRule = db.rules().insert(r -> r.setLanguage(language)); OrganizationDto organization = db.organizations().insert(); @@ -221,18 +222,18 @@ public class RegisterQualityProfilesNotificationTest { } @Test - public void do_not_include_inherited_quality_profile_change_on_existing_rule() { + public void notification_does_not_include_inherited_profiled_when_rule_is_changed() { String language = newLanguageKey(); - RuleDefinitionDto ruleDefinitionDto = db.rules().insert(r -> r.setLanguage(language).setSeverity(Severity.MINOR)); + RuleDefinitionDto rule = db.rules().insert(r -> r.setLanguage(language).setSeverity(Severity.MINOR)); OrganizationDto organization = db.organizations().insert(); QProfileDto builtInQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(true).setLanguage(language)); - ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), builtInQProfileDto); + db.qualityProfiles().activateRule(builtInQProfileDto, rule); QProfileDto childQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(false).setLanguage(language).setParentKee(builtInQProfileDto.getKee())); - ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), childQProfileDto); - db.commit(); - addPluginProfile(builtInQProfileDto, ruleDefinitionDto); + qProfileRules.activateAndCommit(db.getSession(), childQProfileDto, singleton(RuleActivation.create(rule.getKey()))); + addPluginProfile(builtInQProfileDto, rule); builtInQProfileRepositoryRule.initialize(); + db.commit(); underTest.start(); @@ -244,21 +245,21 @@ public class RegisterQualityProfilesNotificationTest { .containsExactlyInAnyOrder(tuple(builtInQProfileDto.getName(), builtInQProfileDto.getLanguage())); assertThat(updatedProfiles.values()) .extracting(value -> value.getActiveRule().getRuleId(), ActiveRuleChange::getType) - .containsExactlyInAnyOrder(tuple(ruleDefinitionDto.getId(), UPDATED)); + .containsExactlyInAnyOrder(tuple(rule.getId(), UPDATED)); } @Test - public void do_not_include_inherited_quality_profile_change_on_deactivated_rule() { + public void notification_does_not_include_inherited_profiles_when_rule_is_deactivated() { String language = newLanguageKey(); - RuleDefinitionDto ruleDefinitionDto = db.rules().insert(r -> r.setLanguage(language).setSeverity(Severity.MINOR)); + RuleDefinitionDto rule = db.rules().insert(r -> r.setLanguage(language).setSeverity(Severity.MINOR)); OrganizationDto organization = db.organizations().insert(); QProfileDto builtInQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(true).setLanguage(language)); - ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), builtInQProfileDto); + db.qualityProfiles().activateRule(builtInQProfileDto, rule); QProfileDto childQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(false).setLanguage(language).setParentKee(builtInQProfileDto.getKee())); - ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), childQProfileDto); + qProfileRules.activateAndCommit(db.getSession(), childQProfileDto, singleton(RuleActivation.create(rule.getKey()))); db.commit(); addPluginProfile(builtInQProfileDto); @@ -274,11 +275,11 @@ public class RegisterQualityProfilesNotificationTest { .containsExactlyInAnyOrder(tuple(builtInQProfileDto.getName(), builtInQProfileDto.getLanguage())); assertThat(updatedProfiles.values()) .extracting(value -> value.getActiveRule().getRuleId(), ActiveRuleChange::getType) - .containsExactlyInAnyOrder(tuple(ruleDefinitionDto.getId(), DEACTIVATED)); + .containsExactlyInAnyOrder(tuple(rule.getId(), DEACTIVATED)); } @Test - public void send_start_and_end_date() { + public void notification_contains_send_start_and_end_date() { String language = newLanguageKey(); RuleDefinitionDto existingRule = db.rules().insert(r -> r.setLanguage(language)); RulesProfileDto dbProfile = insertBuiltInProfile(language); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java index 742ea847029..ab1f04ad2b1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java @@ -20,6 +20,7 @@ package org.sonar.server.qualityprofile.ws; import java.net.HttpURLConnection; +import java.util.Collection; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -40,8 +41,8 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.qualityprofile.RuleActivation; -import org.sonar.server.qualityprofile.RuleActivator; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestResponse; @@ -50,6 +51,7 @@ import org.sonar.server.ws.WsActionTester; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyCollection; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.sonar.server.platform.db.migration.def.VarcharColumnDef.UUID_SIZE; @@ -66,10 +68,10 @@ public class ActivateRuleActionTest { public ExpectedException expectedException = ExpectedException.none(); private DbClient dbClient = db.getDbClient(); - private RuleActivator ruleActivator = mock(RuleActivator.class); + private QProfileRules qProfileRules = mock(QProfileRules.class); private QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSession, TestDefaultOrganizationProvider.from(db)); - private WsActionTester ws = new WsActionTester(new ActivateRuleAction(dbClient, ruleActivator, userSession, wsSupport)); + private WsActionTester ws = new WsActionTester(new ActivateRuleAction(dbClient, qProfileRules, userSession, wsSupport)); private OrganizationDto defaultOrganization; private OrganizationDto organization; @@ -150,12 +152,17 @@ public class ActivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - ArgumentCaptor captor = ArgumentCaptor.forClass(RuleActivation.class); - verify(ruleActivator).activateAndCommit(any(DbSession.class), captor.capture(), any(QProfileDto.class)); - RuleActivation value = captor.getValue(); - assertThat(value.getRuleKey()).isEqualTo(ruleKey); - assertThat(value.getSeverity()).isEqualTo(Severity.BLOCKER); - assertThat(value.isReset()).isFalse(); + Class> collectionClass = (Class>) (Class) Collection.class; + ArgumentCaptor> ruleActivationCaptor = ArgumentCaptor.forClass(collectionClass); + verify(qProfileRules).activateAndCommit(any(DbSession.class), any(QProfileDto.class), ruleActivationCaptor.capture()); + + Collection activations = ruleActivationCaptor.getValue(); + assertThat(activations).hasSize(1); + + RuleActivation activation = activations.iterator().next(); + assertThat(activation.getRuleKey()).isEqualTo(ruleKey); + assertThat(activation.getSeverity()).isEqualTo(Severity.BLOCKER); + assertThat(activation.isReset()).isFalse(); } @Test @@ -174,11 +181,16 @@ public class ActivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - ArgumentCaptor captor = ArgumentCaptor.forClass(RuleActivation.class); - verify(ruleActivator).activateAndCommit(any(DbSession.class), captor.capture(), any(QProfileDto.class)); - assertThat(captor.getValue().getRuleKey()).isEqualTo(ruleKey); - assertThat(captor.getValue().getSeverity()).isEqualTo(Severity.BLOCKER); - assertThat(captor.getValue().isReset()).isFalse(); + Class> collectionClass = (Class>) (Class) Collection.class; + ArgumentCaptor> ruleActivationCaptor = ArgumentCaptor.forClass(collectionClass); + verify(qProfileRules).activateAndCommit(any(DbSession.class), any(QProfileDto.class), ruleActivationCaptor.capture()); + + Collection activations = ruleActivationCaptor.getValue(); + assertThat(activations).hasSize(1); + RuleActivation activation = activations.iterator().next(); + assertThat(activation.getRuleKey()).isEqualTo(ruleKey); + assertThat(activation.getSeverity()).isEqualTo(Severity.BLOCKER); + assertThat(activation.isReset()).isFalse(); } @Test @@ -198,6 +210,6 @@ public class ActivateRuleActionTest { .setParam("reset", "false") .execute(); - verify(ruleActivator).activateAndCommit(any(DbSession.class), any(RuleActivation.class), any(QProfileDto.class)); + verify(qProfileRules).activateAndCommit(any(DbSession.class), any(QProfileDto.class), anyCollection()); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java index e8fcbeabede..e9ef9ca1247 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java @@ -23,6 +23,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.Mockito; import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; import org.sonar.db.DbTester; @@ -34,7 +35,7 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.rule.ws.RuleQueryFactory; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; @@ -43,7 +44,6 @@ import org.sonar.server.ws.WsActionTester; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_PROFILES; @@ -61,11 +61,11 @@ public class ActivateRulesActionTest { public ExpectedException expectedException = ExpectedException.none(); private DbClient dbClient = db.getDbClient(); - private RuleActivator ruleActivator = mock(RuleActivator.class, RETURNS_DEEP_STUBS); private QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSession, TestDefaultOrganizationProvider.from(db)); private RuleQueryFactory ruleQueryFactory = mock(RuleQueryFactory.class); - private WsActionTester ws = new WsActionTester(new ActivateRulesAction(ruleQueryFactory, userSession, ruleActivator, wsSupport, dbClient)); + private QProfileRules qProfileRules = mock(QProfileRules.class, Mockito.RETURNS_DEEP_STUBS); + private WsActionTester ws = new WsActionTester(new ActivateRulesAction(ruleQueryFactory, userSession, qProfileRules, wsSupport, dbClient)); private OrganizationDto defaultOrganization; private OrganizationDto organization; @@ -120,7 +120,7 @@ public class ActivateRulesActionTest { .setParam(PARAM_TARGET_KEY, qualityProfile.getKee()) .execute(); - verify(ruleActivator).bulkActivateAndCommit(any(), any(), any(), any()); + verify(qProfileRules).bulkActivateAndCommit(any(), any(), any(), any()); } @Test @@ -138,7 +138,7 @@ public class ActivateRulesActionTest { .setParam(PARAM_TARGET_KEY, qualityProfile.getKee()) .execute(); - verify(ruleActivator).bulkActivateAndCommit(any(), any(), any(), any()); + verify(qProfileRules).bulkActivateAndCommit(any(), any(), any(), any()); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ChangeParentActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ChangeParentActionTest.java index ac27660c0ac..89148536290 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ChangeParentActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ChangeParentActionTest.java @@ -52,8 +52,8 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.language.LanguageTesting; import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.qualityprofile.QProfileTreeImpl; import org.sonar.server.qualityprofile.RuleActivator; -import org.sonar.server.qualityprofile.RuleActivatorContextFactory; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; @@ -95,9 +95,9 @@ public class ChangeParentActionTest { private ActiveRuleIndexer activeRuleIndexer; private WsActionTester ws; private OrganizationDto organization; - private RuleActivator ruleActivator; private Language language = LanguageTesting.newLanguage(randomAlphanumeric(20)); private String ruleRepository = randomAlphanumeric(5); + private QProfileTreeImpl qProfileTree; @Before public void setUp() { @@ -107,20 +107,12 @@ public class ChangeParentActionTest { ruleIndex = new RuleIndex(esClient, System2.INSTANCE); ruleIndexer = new RuleIndexer(esClient, dbClient); activeRuleIndexer = new ActiveRuleIndexer(dbClient, esClient); - RuleActivatorContextFactory ruleActivatorContextFactory = new RuleActivatorContextFactory(dbClient); TypeValidations typeValidations = new TypeValidations(Collections.emptyList()); - ruleActivator = new RuleActivator(System2.INSTANCE, dbClient, ruleIndex, ruleActivatorContextFactory, typeValidations, activeRuleIndexer, userSession); - + RuleActivator ruleActivator = new RuleActivator(System2.INSTANCE, dbClient, typeValidations, userSession); + qProfileTree = new QProfileTreeImpl(dbClient, ruleActivator, System2.INSTANCE, activeRuleIndexer); ChangeParentAction underTest = new ChangeParentAction( dbClient, - new RuleActivator( - System2.INSTANCE, - dbClient, - ruleIndex, - ruleActivatorContextFactory, - typeValidations, - activeRuleIndexer, - userSession), + qProfileTree, new Languages(), new QProfileWsSupport( dbClient, @@ -188,7 +180,7 @@ public class ChangeParentActionTest { activeRuleIndexer.indexOnStartup(emptySet()); // Set parent 1 - ruleActivator.setParentAndCommit(dbSession, child, parent1); + qProfileTree.setParentAndCommit(dbSession, child, parent1); // Set parent 2 through WS ws.newRequest() @@ -216,7 +208,7 @@ public class ChangeParentActionTest { activeRuleIndexer.indexOnStartup(emptySet()); // Set parent - ruleActivator.setParentAndCommit(dbSession, child, parent); + qProfileTree.setParentAndCommit(dbSession, child, parent); // Remove parent through WS ws.newRequest() @@ -305,7 +297,7 @@ public class ChangeParentActionTest { assertThat(dbClient.activeRuleDao().selectByProfileUuid(dbSession, child.getKee())).isEmpty(); // Set parent - ruleActivator.setParentAndCommit(dbSession, child, parent); + qProfileTree.setParentAndCommit(dbSession, child, parent); // Remove parent ws.newRequest() @@ -332,7 +324,7 @@ public class ChangeParentActionTest { ruleIndexer.commitAndIndex(dbSession, asList(rule1.getKey(), rule2.getKey())); activeRuleIndexer.indexOnStartup(emptySet()); // Set parent 1 - ruleActivator.setParentAndCommit(dbSession, child, parent1); + qProfileTree.setParentAndCommit(dbSession, child, parent1); UserDto user = db.users().insertUser(); db.qualityProfiles().addUserPermission(child, user); userSession.logIn(user); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/CreateActionTest.java index 48d0dac7458..233d7895b3e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/CreateActionTest.java @@ -50,8 +50,9 @@ import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.qualityprofile.QProfileExporters; import org.sonar.server.qualityprofile.QProfileFactoryImpl; +import org.sonar.server.qualityprofile.QProfileRules; +import org.sonar.server.qualityprofile.QProfileRulesImpl; import org.sonar.server.qualityprofile.RuleActivator; -import org.sonar.server.qualityprofile.RuleActivatorContextFactory; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; @@ -67,7 +68,6 @@ import org.sonarqube.ws.Qualityprofiles.CreateWsResponse; import org.sonarqube.ws.Qualityprofiles.CreateWsResponse.QualityProfile; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_PROFILES; import static org.sonar.server.language.LanguageTesting.newLanguages; @@ -94,9 +94,9 @@ public class CreateActionTest { private RuleIndexer ruleIndexer = new RuleIndexer(es.client(), dbClient); private ActiveRuleIndexer activeRuleIndexer = new ActiveRuleIndexer(dbClient, es.client()); private ProfileImporter[] profileImporters = createImporters(); - private QProfileExporters qProfileExporters = new QProfileExporters(dbClient, null, - new RuleActivator(mock(System2.class), dbClient, ruleIndex, new RuleActivatorContextFactory(dbClient), null, activeRuleIndexer, userSession), - profileImporters); + private RuleActivator ruleActivator = new RuleActivator(System2.INSTANCE, dbClient, null, userSession); + private QProfileRules qProfileRules = new QProfileRulesImpl(dbClient, ruleActivator, ruleIndex, activeRuleIndexer); + private QProfileExporters qProfileExporters = new QProfileExporters(dbClient, null, qProfileRules, profileImporters); private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private CreateAction underTest = new CreateAction(dbClient, new QProfileFactoryImpl(dbClient, UuidFactoryFast.getInstance(), System2.INSTANCE, activeRuleIndexer), diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java index bb711c88f99..037cdcccba9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java @@ -20,6 +20,7 @@ package org.sonar.server.qualityprofile.ws; import java.net.HttpURLConnection; +import java.util.Collection; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -39,7 +40,7 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestResponse; @@ -48,6 +49,7 @@ import org.sonar.server.ws.WsActionTester; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyCollection; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.sonar.server.platform.db.migration.def.VarcharColumnDef.UUID_SIZE; @@ -64,9 +66,9 @@ public class DeactivateRuleActionTest { public ExpectedException expectedException = ExpectedException.none(); private DbClient dbClient = db.getDbClient(); - private RuleActivator ruleActivator = mock(RuleActivator.class); + private QProfileRules qProfileRules = mock(QProfileRules.class); private QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSession, TestDefaultOrganizationProvider.from(db)); - private DeactivateRuleAction underTest = new DeactivateRuleAction(dbClient, ruleActivator, userSession, wsSupport); + private DeactivateRuleAction underTest = new DeactivateRuleAction(dbClient, qProfileRules, userSession, wsSupport); private WsActionTester ws = new WsActionTester(underTest); private OrganizationDto defaultOrganization; private OrganizationDto organization; @@ -102,10 +104,11 @@ public class DeactivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - ArgumentCaptor ruleKeyCaptor = ArgumentCaptor.forClass(RuleKey.class); + Class> collectionClass = (Class>) (Class) Collection.class; + ArgumentCaptor> ruleKeyCaptor = ArgumentCaptor.forClass(collectionClass); ArgumentCaptor qProfileDtoCaptor = ArgumentCaptor.forClass(QProfileDto.class); - verify(ruleActivator).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), ruleKeyCaptor.capture()); - assertThat(ruleKeyCaptor.getValue()).isEqualTo(ruleKey); + verify(qProfileRules).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), ruleKeyCaptor.capture()); + assertThat(ruleKeyCaptor.getValue()).containsExactly(ruleKey); assertThat(qProfileDtoCaptor.getValue().getKee()).isEqualTo(qualityProfile.getKee()); } @@ -123,10 +126,11 @@ public class DeactivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - ArgumentCaptor captor = ArgumentCaptor.forClass(RuleKey.class); + Class> collectionClass = (Class>) (Class) Collection.class; + ArgumentCaptor> ruleKeyCaptor = ArgumentCaptor.forClass(collectionClass); ArgumentCaptor qProfileDtoCaptor = ArgumentCaptor.forClass(QProfileDto.class); - verify(ruleActivator).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), captor.capture()); - assertThat(captor.getValue()).isEqualTo(ruleKey); + verify(qProfileRules).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), ruleKeyCaptor.capture()); + assertThat(ruleKeyCaptor.getValue()).containsExactly(ruleKey); assertThat(qProfileDtoCaptor.getValue().getKee()).isEqualTo(qualityProfile.getKee()); } @@ -145,7 +149,7 @@ public class DeactivateRuleActionTest { .setParam(PARAM_KEY, qualityProfile.getKee()) .execute(); - verify(ruleActivator).deactivateAndCommit(any(DbSession.class), any(QProfileDto.class), any(RuleKey.class)); + verify(qProfileRules).deactivateAndCommit(any(DbSession.class), any(QProfileDto.class), anyCollection()); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java index ed62cc2cac0..dea2b2aa073 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java @@ -34,7 +34,7 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.rule.ws.RuleQueryFactory; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; @@ -61,10 +61,10 @@ public class DeactivateRulesActionTest { public ExpectedException thrown = ExpectedException.none(); private DbClient dbClient = db.getDbClient(); - private RuleActivator ruleActivator = mock(RuleActivator.class, RETURNS_DEEP_STUBS); + private QProfileRules qProfileRules = mock(QProfileRules.class, RETURNS_DEEP_STUBS); private QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSession, TestDefaultOrganizationProvider.from(db)); private RuleQueryFactory ruleQueryFactory = mock(RuleQueryFactory.class); - private DeactivateRulesAction underTest = new DeactivateRulesAction(ruleQueryFactory, userSession, ruleActivator, wsSupport, dbClient); + private DeactivateRulesAction underTest = new DeactivateRulesAction(ruleQueryFactory, userSession, qProfileRules, wsSupport, dbClient); private WsActionTester ws = new WsActionTester(underTest); private OrganizationDto defaultOrganization; private OrganizationDto organization; @@ -117,7 +117,7 @@ public class DeactivateRulesActionTest { .setParam(PARAM_TARGET_KEY, qualityProfile.getKee()) .execute(); - verify(ruleActivator).bulkDeactivateAndCommit(any(), any(), any()); + verify(qProfileRules).bulkDeactivateAndCommit(any(), any(), any()); } @Test @@ -135,7 +135,7 @@ public class DeactivateRulesActionTest { .setParam(PARAM_TARGET_KEY, qualityProfile.getKee()) .execute(); - verify(ruleActivator).bulkDeactivateAndCommit(any(), any(), any()); + verify(qProfileRules).bulkDeactivateAndCommit(any(), any(), any()); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/InheritanceActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/InheritanceActionTest.java index 2b54197918d..7f8d7e50e33 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/InheritanceActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/InheritanceActionTest.java @@ -46,9 +46,12 @@ import org.sonar.server.es.EsTester; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.qualityprofile.QProfileName; +import org.sonar.server.qualityprofile.QProfileRules; +import org.sonar.server.qualityprofile.QProfileRulesImpl; +import org.sonar.server.qualityprofile.QProfileTree; +import org.sonar.server.qualityprofile.QProfileTreeImpl; import org.sonar.server.qualityprofile.RuleActivation; import org.sonar.server.qualityprofile.RuleActivator; -import org.sonar.server.qualityprofile.RuleActivatorContextFactory; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; @@ -59,6 +62,7 @@ import org.sonar.server.ws.WsActionTester; import org.sonarqube.ws.Qualityprofiles.InheritanceWsResponse; import static java.util.Arrays.asList; +import static java.util.Collections.singleton; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.server.qualityprofile.QProfileTesting.newQProfileDto; import static org.sonar.test.JsonAssert.assertJson; @@ -79,7 +83,8 @@ public class InheritanceActionTest { private EsClient esClient; private RuleIndexer ruleIndexer; private ActiveRuleIndexer activeRuleIndexer; - private RuleActivator ruleActivator; + private QProfileRules qProfileRules; + private QProfileTree qProfileTree; private OrganizationDto organization; private InheritanceAction underTest; @@ -97,15 +102,12 @@ public class InheritanceActionTest { dbClient, new QProfileWsSupport(dbClient, userSession, defaultOrganizationProvider), new Languages()); + ws = new WsActionTester(underTest); - ruleActivator = new RuleActivator( - System2.INSTANCE, - dbClient, - new RuleIndex(esClient, System2.INSTANCE), - new RuleActivatorContextFactory(dbClient), - new TypeValidations(new ArrayList<>()), - activeRuleIndexer, - userSession); + RuleIndex ruleIndex = new RuleIndex(esClient, System2.INSTANCE); + RuleActivator ruleActivator = new RuleActivator(System2.INSTANCE, dbClient, new TypeValidations(new ArrayList<>()), userSession); + qProfileRules = new QProfileRulesImpl(dbClient, ruleActivator, ruleIndex, activeRuleIndexer); + qProfileTree = new QProfileTreeImpl(dbClient, ruleActivator, System2.INSTANCE, activeRuleIndexer); organization = dbTester.organizations().insert(); } @@ -251,7 +253,7 @@ public class InheritanceActionTest { } private void setParent(QProfileDto profile, QProfileDto parent) { - ruleActivator.setParentAndCommit(dbSession, parent, profile); + qProfileTree.setParentAndCommit(dbSession, parent, profile); } private RuleDefinitionDto createRule(String lang, String id) { @@ -278,8 +280,8 @@ public class InheritanceActionTest { } private void overrideActiveRuleSeverity(RuleDefinitionDto rule, QProfileDto profile, String severity) { - ruleActivator.activate(dbSession, RuleActivation.create(rule.getKey(), severity, null), profile); - dbSession.commit(); - activeRuleIndexer.indexOnStartup(activeRuleIndexer.getIndexTypes()); + qProfileRules.activateAndCommit(dbSession, profile, singleton(RuleActivation.create(rule.getKey(), severity, null))); +// dbSession.commit(); +// activeRuleIndexer.indexOnStartup(activeRuleIndexer.getIndexTypes()); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java index 89d2392a006..c346772bc6b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java @@ -46,9 +46,10 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.qualityprofile.QProfileName; +import org.sonar.server.qualityprofile.QProfileRules; +import org.sonar.server.qualityprofile.QProfileRulesImpl; import org.sonar.server.qualityprofile.QProfileTesting; import org.sonar.server.qualityprofile.RuleActivator; -import org.sonar.server.qualityprofile.RuleActivatorContextFactory; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; @@ -60,17 +61,17 @@ import org.sonar.server.tester.UserSessionRule; import org.sonar.server.util.TypeValidations; import org.sonar.server.ws.WsActionTester; -import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; +import static org.sonar.server.rule.ws.RulesWsParameters.PARAM_LANGUAGES; +import static org.sonar.server.rule.ws.RulesWsParameters.PARAM_QPROFILE; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_KEY; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_RESET; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_RULE; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_SEVERITY; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_TARGET_KEY; import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_TARGET_SEVERITY; -import static org.sonar.server.rule.ws.RulesWsParameters.PARAM_LANGUAGES; -import static org.sonar.server.rule.ws.RulesWsParameters.PARAM_QPROFILE; public class QProfilesWsMediumTest { @@ -87,19 +88,19 @@ public class QProfilesWsMediumTest { private RuleIndex ruleIndex = new RuleIndex(esTester.client(), System2.INSTANCE); private RuleIndexer ruleIndexer = new RuleIndexer(esTester.client(), dbClient); private ActiveRuleIndexer activeRuleIndexer = new ActiveRuleIndexer(dbClient, esTester.client()); - private RuleActivatorContextFactory ruleActivatorContextFactory = new RuleActivatorContextFactory(dbClient); - private TypeValidations typeValidations = new TypeValidations(asList()); - private RuleActivator ruleActivator = new RuleActivator(System2.INSTANCE, dbClient, ruleIndex, ruleActivatorContextFactory, typeValidations, activeRuleIndexer, userSessionRule); + private TypeValidations typeValidations = new TypeValidations(emptyList()); + private RuleActivator ruleActivator = new RuleActivator(System2.INSTANCE, dbClient, typeValidations, userSessionRule); + private QProfileRules qProfileRules = new QProfileRulesImpl(dbClient, ruleActivator, ruleIndex, activeRuleIndexer); private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(dbTester); private QProfileWsSupport qProfileWsSupport = new QProfileWsSupport(dbClient, userSessionRule, defaultOrganizationProvider); private RuleWsSupport ruleWsSupport = new RuleWsSupport(dbClient, userSessionRule, defaultOrganizationProvider); private RuleQueryFactory ruleQueryFactory = new RuleQueryFactory(dbClient, ruleWsSupport); private OrganizationDto organization; - private WsActionTester wsDeactivateRule = new WsActionTester(new DeactivateRuleAction(dbClient, ruleActivator, userSessionRule, qProfileWsSupport)); - private WsActionTester wsDeactivateRules = new WsActionTester(new DeactivateRulesAction(ruleQueryFactory, userSessionRule, ruleActivator, qProfileWsSupport, dbClient)); - private WsActionTester wsActivateRule = new WsActionTester(new ActivateRuleAction(dbClient, ruleActivator, userSessionRule, qProfileWsSupport)); - private WsActionTester wsActivateRules = new WsActionTester(new ActivateRulesAction(ruleQueryFactory, userSessionRule, ruleActivator, qProfileWsSupport, dbClient)); + private WsActionTester wsDeactivateRule = new WsActionTester(new DeactivateRuleAction(dbClient, qProfileRules, userSessionRule, qProfileWsSupport)); + private WsActionTester wsDeactivateRules = new WsActionTester(new DeactivateRulesAction(ruleQueryFactory, userSessionRule, qProfileRules, qProfileWsSupport, dbClient)); + private WsActionTester wsActivateRule = new WsActionTester(new ActivateRuleAction(dbClient, qProfileRules, userSessionRule, qProfileWsSupport)); + private WsActionTester wsActivateRules = new WsActionTester(new ActivateRulesAction(ruleQueryFactory, userSessionRule, qProfileRules, qProfileWsSupport, dbClient)); @Before public void setUp() throws Exception { @@ -244,7 +245,7 @@ public class QProfilesWsMediumTest { dbSession.clearCache(); fail(); } catch (BadRequestException e) { - assertThat(e.getMessage()).isEqualTo("Rule blah:toto and profile pjava have different languages"); + assertThat(e.getMessage()).isEqualTo("php rule blah:toto cannot be activated on java profile Pjava"); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java index 227359fccc2..fe64919612f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java @@ -49,7 +49,7 @@ import org.sonar.server.es.SearchOptions; import org.sonar.server.organization.OrganizationFlags; import org.sonar.server.organization.TestOrganizationFlags; import org.sonar.server.plugins.ServerPluginRepository; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; @@ -88,7 +88,7 @@ public class RegisterRulesTest { @org.junit.Rule public LogTester logTester = new LogTester(); - private RuleActivator ruleActivator = mock(RuleActivator.class); + private QProfileRules qProfileRules = mock(QProfileRules.class); private WebServerRuleFinder webServerRuleFinder = mock(WebServerRuleFinder.class); private DbClient dbClient = dbTester.getDbClient(); private RuleIndexer ruleIndexer; @@ -535,7 +535,7 @@ public class RegisterRulesTest { when(languages.get("java")).thenReturn(mock(Language.class)); reset(webServerRuleFinder); - RegisterRules task = new RegisterRules(loader, ruleActivator, dbClient, ruleIndexer, activeRuleIndexer, languages, system, organizationFlags, webServerRuleFinder); + RegisterRules task = new RegisterRules(loader, qProfileRules, dbClient, ruleIndexer, activeRuleIndexer, languages, system, organizationFlags, webServerRuleFinder); task.start(); // Execute a commit to refresh session state as the task is using its own session dbTester.getSession().commit(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java index a321e46ce3c..f744707aecf 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/DeleteActionTest.java @@ -34,7 +34,7 @@ import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.rule.index.RuleIndexDefinition; import org.sonar.server.rule.index.RuleIndexer; import org.sonar.server.tester.UserSessionRule; @@ -63,10 +63,10 @@ public class DeleteActionTest { private DbClient dbClient = dbTester.getDbClient(); private DbSession dbSession = dbTester.getSession(); private RuleIndexer ruleIndexer = spy(new RuleIndexer(esTester.client(), dbClient)); - private RuleActivator ruleActivator = mock(RuleActivator.class); + private QProfileRules qProfileRules = mock(QProfileRules.class); private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.fromUuid("ORG1"); private RuleWsSupport ruleWsSupport = new RuleWsSupport(mock(DbClient.class), userSession, defaultOrganizationProvider); - private DeleteAction underTest = new DeleteAction(System2.INSTANCE, ruleIndexer, dbClient, ruleActivator, ruleWsSupport); + private DeleteAction underTest = new DeleteAction(System2.INSTANCE, ruleIndexer, dbClient, qProfileRules, ruleWsSupport); private WsActionTester tester = new WsActionTester(underTest); @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionTest.java index 3a500e1bd5e..f59614b4f03 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.rule.ws; -import java.io.IOException; import java.util.HashSet; import java.util.List; import java.util.function.Consumer; @@ -51,9 +50,10 @@ import org.sonar.server.language.LanguageTesting; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.qualityprofile.ActiveRuleChange; +import org.sonar.server.qualityprofile.QProfileRules; +import org.sonar.server.qualityprofile.QProfileRulesImpl; import org.sonar.server.qualityprofile.RuleActivation; import org.sonar.server.qualityprofile.RuleActivator; -import org.sonar.server.qualityprofile.RuleActivatorContextFactory; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; @@ -72,6 +72,7 @@ import org.sonarqube.ws.Rules.SearchResponse; import static java.util.Arrays.asList; import static java.util.Arrays.stream; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; @@ -115,11 +116,9 @@ public class SearchActionTest { private MacroInterpreter macroInterpreter = mock(MacroInterpreter.class); private RuleMapper ruleMapper = new RuleMapper(languages, macroInterpreter); private SearchAction underTest = new SearchAction(ruleIndex, activeRuleCompleter, ruleQueryFactory, db.getDbClient(), ruleMapper); - - private RuleActivatorContextFactory contextFactory = new RuleActivatorContextFactory(db.getDbClient()); private TypeValidations typeValidations = new TypeValidations(asList(new StringTypeValidation(), new IntegerTypeValidation())); - private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), ruleIndex, contextFactory, typeValidations, activeRuleIndexer, - userSession); + private RuleActivator ruleActivator = new RuleActivator(System2.INSTANCE, db.getDbClient(), typeValidations, userSession); + private QProfileRules qProfileRules = new QProfileRulesImpl(db.getDbClient(), ruleActivator, ruleIndex, activeRuleIndexer); private WsActionTester ws = new WsActionTester(underTest); @Before @@ -615,7 +614,7 @@ public class SearchActionTest { QProfileDto profile = db.qualityProfiles().insert(organization, p -> p.setLanguage("java")); RuleDefinitionDto rule = createJavaRule(); RuleActivation activation = RuleActivation.create(rule.getKey(), BLOCKER, null); - ruleActivator.activate(db.getSession(), activation, profile); + qProfileRules.activateAndCommit(db.getSession(), profile, singleton(activation)); indexRules(); @@ -662,8 +661,8 @@ public class SearchActionTest { .setName("empty_var")); RuleActivation activation = RuleActivation.create(rule.getKey()); - List activeRuleChanges1 = ruleActivator.activate(db.getSession(), activation, profile); - ruleActivator.activate(db.getSession(), activation, waterproofProfile); + List activeRuleChanges1 = qProfileRules.activateAndCommit(db.getSession(), profile, singleton(activation)); + qProfileRules.activateAndCommit(db.getSession(), waterproofProfile, singleton(activation)); assertThat(activeRuleChanges1).hasSize(1); @@ -718,7 +717,7 @@ public class SearchActionTest { .setName("my_var")); RuleActivation activation = RuleActivation.create(rule.getKey()); - List activeRuleChanges = ruleActivator.activate(db.getSession(), activation, profile); + List activeRuleChanges = qProfileRules.activateAndCommit(db.getSession(), profile, singleton(activation)); // Insert directly in database a rule parameter with a null value ActiveRuleParamDto activeRuleParam = ActiveRuleParamDto.createFor(ruleParam).setValue(null); @@ -781,7 +780,7 @@ public class SearchActionTest { .setStatus(RuleStatus.DEPRECATED) .setType(RuleType.VULNERABILITY)); RuleActivation activation = RuleActivation.create(rule2.getKey(), null, null); - ruleActivator.activate(db.getSession(), activation, profile); + qProfileRules.activateAndCommit(db.getSession(), profile, singleton(activation)); // on other language, not activated => no match RuleDefinitionDto rule3 = db.rules().insert(r -> r -- 2.39.5