diff options
7 files changed, 108 insertions, 66 deletions
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 51caaf68599..b711c69c15b 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 @@ -47,32 +47,33 @@ public class BuiltInQProfileUpdateImpl implements BuiltInQProfileUpdate { this.activeRuleIndexer = activeRuleIndexer; } - public List<ActiveRuleChange> update(DbSession dbSession, BuiltInQProfile builtIn, RulesProfileDto rulesProfile) { + public List<ActiveRuleChange> update(DbSession dbSession, BuiltInQProfile builtInDefinition, RulesProfileDto initialRuleProfile) { // Keep reference to all the activated rules before update - Set<Integer> deactivatedRuleIds = dbClient.activeRuleDao().selectByRuleProfile(dbSession, rulesProfile) + Set<Integer> deactivatedRuleIds = dbClient.activeRuleDao().selectByRuleProfile(dbSession, initialRuleProfile) .stream() .map(ActiveRuleDto::getRuleId) .collect(MoreCollectors.toHashSet()); - Set<Integer> ruleKeys = Stream.concat( + // all rules, including those which are removed from built-in profile + Set<Integer> ruleIds = Stream.concat( deactivatedRuleIds.stream(), - builtIn.getActiveRules().stream().map(BuiltInQProfile.ActiveRule::getRuleId)) + builtInDefinition.getActiveRules().stream().map(BuiltInQProfile.ActiveRule::getRuleId)) .collect(toSet()); - RuleActivationContext context = ruleActivator.createContextForBuiltInProfile(dbSession, rulesProfile, ruleKeys); Collection<RuleActivation> activations = new ArrayList<>(); - for (BuiltInQProfile.ActiveRule ar : builtIn.getActiveRules()) { + for (BuiltInQProfile.ActiveRule ar : builtInDefinition.getActiveRules()) { RuleActivation activation = convert(ar); activations.add(activation); deactivatedRuleIds.remove(activation.getRuleId()); } + RuleActivationContext context = ruleActivator.createContextForBuiltInProfile(dbSession, initialRuleProfile, ruleIds); List<ActiveRuleChange> 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 + // these rules are no longer part of the built-in profile deactivatedRuleIds.forEach(ruleKey -> changes.addAll(ruleActivator.deactivate(dbSession, context, ruleKey, false))); activeRuleIndexer.commitAndIndex(dbSession, changes); 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 2d940a3e805..79ec59531e8 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 @@ -62,9 +62,10 @@ public class QProfileResetImpl implements QProfileReset { rulesToBeDeactivated.add(activeRuleDto.getRuleId()); } } - Set<Integer> ruleKeys = new HashSet<>(rulesToBeDeactivated); - activations.forEach(a -> ruleKeys.add(a.getRuleId())); - RuleActivationContext context = activator.createContextForUserProfile(dbSession, profile, ruleKeys); + Set<Integer> ruleIds = new HashSet<>(rulesToBeDeactivated.size() + activations.size()); + ruleIds.addAll(rulesToBeDeactivated); + activations.forEach(a -> ruleIds.add(a.getRuleId())); + RuleActivationContext context = activator.createContextForUserProfile(dbSession, profile, ruleIds); for (RuleActivation activation : activations) { try { 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 cfbbc2b1e7f..a23dd2f5ce6 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 @@ -366,7 +366,7 @@ public class RuleActivator { // load rules List<RuleDefinitionDto> rules = completeWithRules(dbSession, builder, ruleIds); - // load profiles + // load org profiles List<QProfileDto> aliasedBuiltInProfiles = db.qualityProfileDao().selectQProfilesByRuleProfile(dbSession, builtInProfile); List<QProfileDto> profiles = new ArrayList<>(aliasedBuiltInProfiles); profiles.addAll(db.qualityProfileDao().selectDescendants(dbSession, aliasedBuiltInProfiles)); 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 889a5abbf47..654a0b86221 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 @@ -20,7 +20,9 @@ package org.sonar.server.qualityprofile; import java.util.List; +import java.util.Map; import java.util.Optional; +import javax.annotation.Nullable; import org.assertj.core.groups.Tuple; import org.junit.Before; import org.junit.Rule; @@ -34,6 +36,8 @@ import org.sonar.api.utils.internal.TestSystem2; import org.sonar.db.DbTester; import org.sonar.db.qualityprofile.ActiveRuleDto; 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.RuleParamDto; @@ -44,6 +48,7 @@ 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 org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.groups.Tuple.tuple; import static org.mockito.Mockito.mock; @@ -52,6 +57,7 @@ import static org.sonar.api.rules.RulePriority.CRITICAL; import static org.sonar.api.rules.RulePriority.MAJOR; import static org.sonar.api.rules.RulePriority.MINOR; import static org.sonar.db.qualityprofile.QualityProfileTesting.newRuleProfileDto; +import static org.sonar.server.qualityprofile.ActiveRuleInheritance.INHERITED; public class BuiltInQProfileUpdateImplTest { @@ -61,7 +67,7 @@ public class BuiltInQProfileUpdateImplTest { @Rule public BuiltInQProfileRepositoryRule builtInProfileRepository = new BuiltInQProfileRepositoryRule(); @Rule - public DbTester db = DbTester.create().setDisableDefaultOrganization(true); + public DbTester db = DbTester.create(); @Rule public UserSessionRule userSession = UserSessionRule.standalone(); private System2 system2 = new TestSystem2().setNow(NOW); @@ -222,10 +228,10 @@ public class BuiltInQProfileUpdateImplTest { RuleParamDto ruleParam = db.rules().insertRuleParam(rule, p -> p.setName("min").setDefaultValue("10")); BuiltInQualityProfilesDefinition.Context context = new BuiltInQualityProfilesDefinition.Context(); - NewBuiltInQualityProfile newQp = context.createBuiltInQualityProfile("Sonar way", "xoo"); + NewBuiltInQualityProfile newQp = context.createBuiltInQualityProfile("Sonar way", rule.getLanguage()); newQp.activateRule(rule.getRepositoryKey(), rule.getRuleKey()); newQp.done(); - BuiltInQProfile builtIn = builtInProfileRepository.create(context.profile("xoo", "Sonar way"), rule); + BuiltInQProfile builtIn = builtInProfileRepository.create(context.profile(newQp.language(), newQp.name()), rule); underTest.update(db.getSession(), builtIn, persistedProfile); List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile); @@ -242,47 +248,86 @@ public class BuiltInQProfileUpdateImplTest { assertThatRuleHasParams(db, activeRules.get(0), tuple("min", "20")); } -// @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<ActiveRuleChange> 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<ActiveRuleChange> 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 propagate_new_activation_to_descendant_profiles() { + RuleDefinitionDto rule = db.rules().insert(r -> r.setLanguage("xoo")); + + QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), + p -> p.setLanguage(rule.getLanguage()).setIsBuiltIn(true)); + QProfileDto childProfile = createChildProfile(profile); + QProfileDto grandchildProfile = createChildProfile(childProfile); + + BuiltInQualityProfilesDefinition.Context context = new BuiltInQualityProfilesDefinition.Context(); + NewBuiltInQualityProfile newQp = context.createBuiltInQualityProfile(profile.getName(), profile.getLanguage()); + newQp.activateRule(rule.getRepositoryKey(), rule.getRuleKey()); + newQp.done(); + BuiltInQProfile builtIn = builtInProfileRepository.create(context.profile(profile.getLanguage(), profile.getName()), rule); + List<ActiveRuleChange> changes = underTest.update(db.getSession(), builtIn, 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()); + } + + private QProfileDto createChildProfile(QProfileDto parent) { + return db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p + .setLanguage(parent.getLanguage()) + .setParentKee(parent.getKee()) + .setName("Child of " + parent.getName())) + .setIsBuiltIn(false); + } + + private void assertThatRuleIsActivated(QProfileDto profile, RuleDefinitionDto rule, @Nullable List<ActiveRuleChange> changes, + String expectedSeverity, @Nullable ActiveRuleInheritance expectedInheritance, Map<String, String> 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<ActiveRuleParamDto> 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); + } + } + + // + // @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<ActiveRuleChange> 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 assertThatRuleHasParams(DbTester db, ActiveRuleDto activeRule, Tuple... expectedParams) { assertThat(db.getDbClient().activeRuleDao().selectParamsByActiveRuleId(db.getSession(), activeRule.getId())) diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleImplTest.java index 5bf7e972b45..d50cd3085fa 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleImplTest.java @@ -893,7 +893,7 @@ public class QProfileRuleImplTest { } private void assertThatRuleIsActivated(QProfileDto profile, RuleDefinitionDto rule, @Nullable List<ActiveRuleChange> changes, - String expectedSeverity, @Nullable ActiveRuleInheritance expectedInheritance, Map<String, String> expectedParams) { + String expectedSeverity, @Nullable ActiveRuleInheritance expectedInheritance, Map<String, String> expectedParams) { OrgActiveRuleDto activeRule = db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile) .stream() .filter(ar -> ar.getRuleKey().equals(rule.getKey())) @@ -928,7 +928,7 @@ public class QProfileRuleImplTest { } private void assertThatRuleIsUpdated(QProfileDto profile, RuleDefinitionDto rule, - String expectedSeverity, @Nullable ActiveRuleInheritance expectedInheritance, Map<String, String> expectedParams) { + String expectedSeverity, @Nullable ActiveRuleInheritance expectedInheritance, Map<String, String> expectedParams) { OrgActiveRuleDto activeRule = db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile) .stream() .filter(ar -> ar.getRuleKey().equals(rule.getKey())) 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 be696b3a1b3..fe991ff0a71 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 @@ -411,13 +411,13 @@ public class QProfilesWsMediumTest { @Test public void reset() { QProfileDto profile = QProfileTesting.newXooP1(organization); - QProfileDto subProfile = QProfileTesting.newXooP2(organization).setParentKee(QProfileTesting.XOO_P1_KEY); - dbClient.qualityProfileDao().insert(dbSession, profile, subProfile); + QProfileDto childProfile = QProfileTesting.newXooP2(organization).setParentKee(QProfileTesting.XOO_P1_KEY); + dbClient.qualityProfileDao().insert(dbSession, profile, childProfile); RuleDefinitionDto rule = createRule(profile.getLanguage(), "rule"); ActiveRuleDto active1 = ActiveRuleDto.createFor(profile, rule) .setSeverity(rule.getSeverityString()); - ActiveRuleDto active2 = ActiveRuleDto.createFor(subProfile, rule) + ActiveRuleDto active2 = ActiveRuleDto.createFor(childProfile, rule) .setSeverity("MINOR"); dbClient.activeRuleDao().insert(dbSession, active1); dbClient.activeRuleDao().insert(dbSession, active2); @@ -432,7 +432,7 @@ public class QProfilesWsMediumTest { // 1. reset child rule wsActivateRule.newRequest().setMethod("POST") - .setParam(PARAM_KEY, subProfile.getKee()) + .setParam(PARAM_KEY, childProfile.getKee()) .setParam(PARAM_RULE, rule.getKey().toString()) .setParam(PARAM_RESET, "true") .execute(); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/profile/BuiltInQualityProfilesDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/profile/BuiltInQualityProfilesDefinition.java index b3016cbde10..71b062c4b61 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/profile/BuiltInQualityProfilesDefinition.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/profile/BuiltInQualityProfilesDefinition.java @@ -307,11 +307,6 @@ public interface BuiltInQualityProfilesDefinition { return param; } - @CheckForNull - public NewOverriddenParam getOverriddenParam(String paramKey) { - return paramsByKey.get(paramKey); - } - public Collection<NewOverriddenParam> getOverriddenParams() { return paramsByKey.values(); } |