From: Dejan Milisavljevic Date: Tue, 15 Oct 2024 07:14:54 +0000 (+0200) Subject: SONAR-23250 Compare active rules impacts when comparing quality profiles X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c23b714596ae2a0f551a32931a34890197c1d4a9;p=sonarqube.git SONAR-23250 Compare active rules impacts when comparing quality profiles --- diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileComparisonIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileComparisonIT.java index 4649c782c51..eb8750c96dc 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileComparisonIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileComparisonIT.java @@ -21,11 +21,13 @@ package org.sonar.server.qualityprofile; import com.google.common.collect.ImmutableMap; import com.google.common.collect.MapDifference.ValueDifference; +import java.util.List; +import java.util.Map; import org.assertj.core.data.MapEntry; -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.sonar.api.config.Configuration; import org.sonar.api.rule.Severity; import org.sonar.api.server.rule.RuleParamType; @@ -35,6 +37,7 @@ import org.sonar.core.platform.SonarQubeVersion; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; +import org.sonar.db.issue.ImpactDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.qualityprofile.QualityProfileTesting; import org.sonar.db.rule.RuleDto; @@ -55,15 +58,20 @@ import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.sonar.api.issue.impact.Severity.BLOCKER; +import static org.sonar.api.issue.impact.Severity.LOW; +import static org.sonar.api.issue.impact.Severity.MEDIUM; +import static org.sonar.api.issue.impact.SoftwareQuality.MAINTAINABILITY; +import static org.sonar.api.issue.impact.SoftwareQuality.SECURITY; -public class QProfileComparisonIT { +class QProfileComparisonIT { - @Rule - public UserSessionRule userSession = UserSessionRule.standalone().anonymous(); - @Rule - public DbTester dbTester = DbTester.create(); - @Rule - public EsTester es = EsTester.create(); + @RegisterExtension + private final UserSessionRule userSession = UserSessionRule.standalone().anonymous(); + @RegisterExtension + private final DbTester dbTester = DbTester.create(); + @RegisterExtension + private final EsTester es = EsTester.create(); private DbSession dbSession; private QProfileRules qProfileRules; @@ -74,8 +82,8 @@ public class QProfileComparisonIT { private QProfileDto left; private QProfileDto right; - @Before - public void before() { + @BeforeEach + void before() { DbClient db = dbTester.getDbClient(); dbSession = db.openSession(false); RuleIndex ruleIndex = new RuleIndex(es.client(), System2.INSTANCE); @@ -87,8 +95,8 @@ public class QProfileComparisonIT { qProfileRules = new QProfileRulesImpl(db, ruleActivator, ruleIndex, activeRuleIndexer, qualityProfileChangeEventService); comparison = new QProfileComparison(db); - xooRule1 = RuleTesting.newXooX1().setSeverity("MINOR"); - xooRule2 = RuleTesting.newXooX2().setSeverity("MAJOR"); + xooRule1 = RuleTesting.newXooX1().setSeverity("MINOR").replaceAllDefaultImpacts(List.of(new ImpactDto(SECURITY, LOW))); + xooRule2 = RuleTesting.newXooX2().setSeverity("MAJOR").replaceAllDefaultImpacts(List.of(new ImpactDto(MAINTAINABILITY, BLOCKER))); db.ruleDao().insert(dbSession, xooRule1); db.ruleDao().insert(dbSession, xooRule2); db.ruleDao().insertRuleParam(dbSession, xooRule1, RuleParamDto.createFor(xooRule1) @@ -103,13 +111,13 @@ public class QProfileComparisonIT { dbSession.commit(); } - @After - public void after() { + @AfterEach + void after() { dbSession.close(); } @Test - public void compare_empty_profiles() { + void compare_empty_profiles() { QProfileComparisonResult result = comparison.compare(dbSession, left, right); assertThat(result.left().getKee()).isEqualTo(left.getKee()); assertThat(result.right().getKee()).isEqualTo(right.getKee()); @@ -117,12 +125,14 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isEmpty(); assertThat(result.inRight()).isEmpty(); assertThat(result.modified()).isEmpty(); - assertThat(result.collectRuleKeys()).isEmpty(); + assertThat(result.getImpactedRules()).isEmpty(); } @Test - public void compare_same() { + void compare_same() { RuleActivation commonActivation = RuleActivation.create(xooRule1.getUuid(), Severity.CRITICAL, + Map.of(SECURITY, BLOCKER), + false, ImmutableMap.of("min", "7", "max", "42")); qProfileRules.activateAndCommit(dbSession, left, singleton(commonActivation)); qProfileRules.activateAndCommit(dbSession, right, singleton(commonActivation)); @@ -134,11 +144,11 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isEmpty(); assertThat(result.inRight()).isEmpty(); assertThat(result.modified()).isEmpty(); - assertThat(result.collectRuleKeys()).containsOnly(xooRule1.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey()); } @Test - public void compare_only_left() { + void compare_only_left() { RuleActivation activation = RuleActivation.create(xooRule1.getUuid()); qProfileRules.activateAndCommit(dbSession, left, singleton(activation)); @@ -149,11 +159,11 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isNotEmpty().containsOnlyKeys(xooRule1.getKey()); assertThat(result.inRight()).isEmpty(); assertThat(result.modified()).isEmpty(); - assertThat(result.collectRuleKeys()).containsOnly(xooRule1.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey()); } @Test - public void compare_only_right() { + void compare_only_right() { qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getUuid()))); QProfileComparisonResult result = comparison.compare(dbSession, left, right); @@ -163,11 +173,11 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isEmpty(); assertThat(result.inRight()).isNotEmpty().containsOnlyKeys(xooRule1.getKey()); assertThat(result.modified()).isEmpty(); - assertThat(result.collectRuleKeys()).containsOnly(xooRule1.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey()); } @Test - public void compare_disjoint() { + void compare_disjoint() { qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getUuid()))); qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule2.getUuid()))); @@ -178,11 +188,11 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isNotEmpty().containsOnlyKeys(xooRule1.getKey()); assertThat(result.inRight()).isNotEmpty().containsOnlyKeys(xooRule2.getKey()); assertThat(result.modified()).isEmpty(); - assertThat(result.collectRuleKeys()).containsOnly(xooRule1.getKey(), xooRule2.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey(), xooRule2.getKey()); } @Test - public void compare_modified_severity() { + void compare_modified_severity() { qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getUuid(), Severity.CRITICAL, null))); qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getUuid(), Severity.BLOCKER, null))); @@ -193,7 +203,7 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isEmpty(); assertThat(result.inRight()).isEmpty(); assertThat(result.modified()).isNotEmpty().containsOnlyKeys(xooRule1.getKey()); - assertThat(result.collectRuleKeys()).containsOnly(xooRule1.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey()); ActiveRuleDiff activeRuleDiff = result.modified().get(xooRule1.getKey()); assertThat(activeRuleDiff.leftSeverity()).isEqualTo(Severity.CRITICAL); @@ -202,7 +212,32 @@ public class QProfileComparisonIT { } @Test - public void compare_modified_param() { + void compare_modified_impacts() { + qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getUuid(), null, + Map.of(SECURITY, BLOCKER), null, null))); + qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getUuid(), null, + Map.of(SECURITY, MEDIUM), null, null))); + + QProfileComparisonResult result = comparison.compare(dbSession, left, right); + assertThat(result.left().getKee()).isEqualTo(left.getKee()); + assertThat(result.right().getKee()).isEqualTo(right.getKee()); + assertThat(result.same()).isEmpty(); + assertThat(result.inLeft()).isEmpty(); + assertThat(result.inRight()).isEmpty(); + assertThat(result.modified()).isNotEmpty().containsOnlyKeys(xooRule1.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey()); + + ActiveRuleDiff activeRuleDiff = result.modified().get(xooRule1.getKey()); + assertThat(activeRuleDiff.impactDifference().entriesDiffering()).isNotEmpty(); + ValueDifference impactdiff = activeRuleDiff.impactDifference().entriesDiffering().get(SECURITY); + assertThat(impactdiff.leftValue()).isEqualTo(BLOCKER); + assertThat(impactdiff.rightValue()).isEqualTo(MEDIUM); + + assertThat(activeRuleDiff.paramDifference().areEqual()).isTrue(); + } + + @Test + void compare_modified_param() { qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getUuid(), null, ImmutableMap.of("max", "20")))); qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getUuid(), null, ImmutableMap.of("max", "30")))); @@ -213,7 +248,7 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isEmpty(); assertThat(result.inRight()).isEmpty(); assertThat(result.modified()).isNotEmpty().containsOnlyKeys(xooRule1.getKey()); - assertThat(result.collectRuleKeys()).containsOnly(xooRule1.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey()); ActiveRuleDiff activeRuleDiff = result.modified().get(xooRule1.getKey()); assertThat(activeRuleDiff.leftSeverity()).isEqualTo(activeRuleDiff.rightSeverity()).isEqualTo(xooRule1.getSeverityString()); @@ -225,7 +260,7 @@ public class QProfileComparisonIT { } @Test - public void compare_different_params() { + void compare_different_params() { qProfileRules.activateAndCommit(dbSession, left, singleton(RuleActivation.create(xooRule1.getUuid(), null, ImmutableMap.of("max", "20")))); qProfileRules.activateAndCommit(dbSession, right, singleton(RuleActivation.create(xooRule1.getUuid(), null, ImmutableMap.of("min", "5")))); @@ -236,7 +271,7 @@ public class QProfileComparisonIT { assertThat(result.inLeft()).isEmpty(); assertThat(result.inRight()).isEmpty(); assertThat(result.modified()).isNotEmpty().containsOnlyKeys(xooRule1.getKey()); - assertThat(result.collectRuleKeys()).containsOnly(xooRule1.getKey()); + assertThat(result.getImpactedRules()).containsOnlyKeys(xooRule1.getKey()); ActiveRuleDiff activeRuleDiff = result.modified().get(xooRule1.getKey()); assertThat(activeRuleDiff.leftSeverity()).isEqualTo(activeRuleDiff.rightSeverity()).isEqualTo(xooRule1.getSeverityString()); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/ws/CompareActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/ws/CompareActionIT.java index 6fed4869d8e..51db5f8ae47 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/ws/CompareActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/ws/CompareActionIT.java @@ -19,9 +19,12 @@ */ package org.sonar.server.qualityprofile.ws; +import java.util.Collection; +import java.util.List; +import java.util.Map; import org.apache.commons.lang3.StringUtils; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.resources.Languages; import org.sonar.api.rule.RuleKey; @@ -45,22 +48,22 @@ import org.sonar.db.rule.RuleParamDto; import org.sonar.db.rule.RuleRepositoryDto; import org.sonar.server.language.LanguageTesting; import org.sonar.server.qualityprofile.QProfileComparison; -import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsActionTester; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.sonar.api.issue.impact.Severity.BLOCKER; import static org.sonar.api.issue.impact.Severity.HIGH; +import static org.sonar.api.issue.impact.Severity.LOW; +import static org.sonar.api.issue.impact.Severity.MEDIUM; +import static org.sonar.api.issue.impact.SoftwareQuality.RELIABILITY; +import static org.sonar.api.issue.impact.SoftwareQuality.SECURITY; -public class CompareActionIT { +class CompareActionIT { - @Rule - public DbTester db = DbTester.create(); - @Rule - public UserSessionRule userSessionRule = UserSessionRule.standalone(); - @Rule - public UserSessionRule userSession = UserSessionRule.standalone(); + @RegisterExtension + private final DbTester db = DbTester.create(); private final DbClient dbClient = db.getDbClient(); private final DbSession session = db.getSession(); @@ -69,7 +72,7 @@ public class CompareActionIT { new CompareAction(db.getDbClient(), new QProfileComparison(db.getDbClient()), new Languages(LanguageTesting.newLanguage("xoo", "Xoo")))); @Test - public void compare_nominal() { + void compare_nominal() { createRepository("blah", "xoo", "Blah"); RuleDto rule1 = createRule("xoo", "rule1"); @@ -77,6 +80,7 @@ public class CompareActionIT { RuleDto rule3 = createRule("xoo", "rule3"); RuleDto rule4 = createRuleWithParam("xoo", "rule4"); RuleDto rule5 = createRule("xoo", "rule5"); + RuleDto rule6 = createRuleWithImpacts("xoo", "rule6", List.of(new ImpactDto(SECURITY, BLOCKER))); /* * Profile 1: @@ -84,12 +88,14 @@ public class CompareActionIT { * - rule 2 active (only in this profile) => "inLeft" * - rule 4 active with different parameters => "modified" * - rule 5 active with different severity => "modified" + * - rule 6 active with different impacts => "modified" */ QProfileDto profile1 = createProfile("xoo", "Profile 1", "xoo-profile-1-01234"); createActiveRule(rule1, profile1); createActiveRule(rule2, profile1); createActiveRuleWithParam(rule4, profile1, "polop"); createActiveRuleWithSeverity(rule5, profile1, Severity.MINOR); + createActiveRuleWithImpacts(rule6, profile1, Map.of(SECURITY, BLOCKER)); session.commit(); /* @@ -97,12 +103,14 @@ public class CompareActionIT { * - rule 1 active (on both profiles) => "same" * - rule 3 active (only in this profile) => "inRight" * - rule 4 active with different parameters => "modified" + * - rule 6 active with different impacts => "modified" */ QProfileDto profile2 = createProfile("xoo", "Profile 2", "xoo-profile-2-12345"); createActiveRule(rule1, profile2); createActiveRule(rule3, profile2); createActiveRuleWithParam(rule4, profile2, "palap"); createActiveRuleWithSeverity(rule5, profile2, Severity.MAJOR); + createActiveRuleWithImpacts(rule6, profile2, Map.of(SECURITY, HIGH)); session.commit(); ws.newRequest() @@ -112,7 +120,7 @@ public class CompareActionIT { } @Test - public void compare_whenSecurityHotspot_shouldReturnEmptyCleanCodeInformation() { + void compare_whenSecurityHotspot_shouldReturnEmptyCleanCodeInformation() { createRepository("blah", "xoo", "Blah"); RuleDto rule1 = createSecurityHotspot("xoo", "rule1"); @@ -132,7 +140,7 @@ public class CompareActionIT { } @Test - public void compare_param_on_left() { + void compare_param_on_left() { RuleDto rule1 = createRuleWithParam("xoo", "rule1"); createRepository("blah", "xoo", "Blah"); QProfileDto profile1 = createProfile("xoo", "Profile 1", "xoo-profile-1-01234"); @@ -148,7 +156,7 @@ public class CompareActionIT { } @Test - public void compare_param_on_right() { + void compare_param_on_right() { RuleDto rule1 = createRuleWithParam("xoo", "rule1"); createRepository("blah", "xoo", "Blah"); QProfileDto profile1 = createProfile("xoo", "Profile 1", "xoo-profile-1-01234"); @@ -164,7 +172,38 @@ public class CompareActionIT { } @Test - public void fail_on_missing_left_param() { + void compare_impacts() { + RuleDto rule1 = createRuleWithImpacts("xoo", "impactRule", List.of(new ImpactDto(SECURITY, MEDIUM))); + createRepository("blah", "xoo", "Blah"); + QProfileDto profile1 = createProfile("xoo", "Profile 1", "xoo-profile-1-01234"); + createActiveRuleWithImpacts(rule1, profile1, Map.of(SECURITY, MEDIUM, RELIABILITY, HIGH)); + QProfileDto profile2 = createProfile("xoo", "Profile 2", "xoo-profile-2-12345"); + createActiveRuleWithImpacts(rule1, profile2, Map.of(SECURITY, BLOCKER)); + session.commit(); + + ws.newRequest() + .setParam("leftKey", profile1.getKee()) + .setParam("rightKey", profile2.getKee()) + .execute().assertJson(this.getClass(), "compare_impacts.json"); + } + + @Test + void compare_impacts_left_only_should_return_impacts_based_on_rule_impacts() { + RuleDto rule1 = createRuleWithImpacts("xoo", "impactRule", List.of(new ImpactDto(SECURITY, MEDIUM))); + createRepository("blah", "xoo", "Blah"); + QProfileDto profile1 = createProfile("xoo", "Profile 1", "xoo-profile-1-01234"); + createActiveRuleWithImpacts(rule1, profile1, Map.of(SECURITY, LOW, RELIABILITY, HIGH)); + QProfileDto profile2 = createProfile("xoo", "Profile 2", "xoo-profile-2-12345"); + session.commit(); + + ws.newRequest() + .setParam("leftKey", profile1.getKee()) + .setParam("rightKey", profile2.getKee()) + .execute().assertJson(this.getClass(), "compare_impacts_left_only.json"); + } + + @Test + void fail_on_missing_left_param() { assertThatThrownBy(() -> { ws.newRequest() .setParam("rightKey", "polop") @@ -174,7 +213,7 @@ public class CompareActionIT { } @Test - public void fail_on_missing_right_param() { + void fail_on_missing_right_param() { assertThatThrownBy(() -> { ws.newRequest() .setParam("leftKey", "polop") @@ -184,7 +223,7 @@ public class CompareActionIT { } @Test - public void fail_on_left_profile_not_found() { + void fail_on_left_profile_not_found() { createProfile("xoo", "Right", "xoo-right-12345"); assertThatThrownBy(() -> { @@ -197,7 +236,7 @@ public class CompareActionIT { } @Test - public void fail_on_right_profile_not_found() { + void fail_on_right_profile_not_found() { createProfile("xoo", "Left", "xoo-left-12345"); assertThatThrownBy(() -> { @@ -210,7 +249,7 @@ public class CompareActionIT { } @Test - public void definition() { + void definition() { WebService.Action definition = ws.getDef(); assertThat(definition).isNotNull(); assertThat(definition.isPost()).isFalse(); @@ -239,6 +278,21 @@ public class CompareActionIT { return ruleDto; } + private RuleDto createRuleWithImpacts(String lang, String id, Collection impacts) { + RuleDto rule = createFor(RuleKey.of("blah", id)) + .setUuid(Uuids.createFast()) + .setName(StringUtils.capitalize(id)) + .setLanguage(lang) + .setSeverity(Severity.BLOCKER) + .setScope(Scope.MAIN) + .setStatus(RuleStatus.READY) + .setCleanCodeAttribute(CleanCodeAttribute.EFFICIENT) + .replaceAllDefaultImpacts(impacts); + RuleDto ruleDto = rule; + dbClient.ruleDao().insert(session, ruleDto); + return ruleDto; + } + private RuleDto createSecurityHotspot(String lang, String id) { RuleDto rule = createFor(RuleKey.of("blah", id)) .setUuid(Uuids.createFast()) @@ -269,7 +323,17 @@ public class CompareActionIT { private ActiveRuleDto createActiveRule(RuleDto rule, QProfileDto profile) { ActiveRuleDto activeRule = ActiveRuleDto.createFor(profile, rule) - .setSeverity(rule.getSeverityString()); + .setSeverity(rule.getSeverityString()) + .setImpacts(rule.getDefaultImpactsMap()); + dbClient.activeRuleDao().insert(session, activeRule); + return activeRule; + } + + private ActiveRuleDto createActiveRuleWithImpacts(RuleDto rule, QProfileDto profile, Map impacts) { + ActiveRuleDto activeRule = ActiveRuleDto.createFor(profile, rule) + .setSeverity(rule.getSeverityString()) + .setImpacts(impacts); dbClient.activeRuleDao().insert(session, activeRule); return activeRule; } @@ -284,7 +348,8 @@ public class CompareActionIT { private ActiveRuleDto createActiveRuleWithSeverity(RuleDto rule, QProfileDto profile, String severity) { ActiveRuleDto activeRule = ActiveRuleDto.createFor(profile, rule) - .setSeverity(severity); + .setSeverity(severity) + .setImpacts(rule.getDefaultImpactsMap()); dbClient.activeRuleDao().insert(session, activeRule); return activeRule; } diff --git a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_impacts.json b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_impacts.json new file mode 100644 index 00000000000..a1a201e73a0 --- /dev/null +++ b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_impacts.json @@ -0,0 +1,45 @@ +{ + "left": { + "key": "xoo-profile-1-01234", + "name": "Profile 1" + }, + "right": { + "key": "xoo-profile-2-12345", + "name": "Profile 2" + }, + "inLeft": [], + "inRight": [], + "modified": [ + { + "key": "blah:impactRule", + "name": "ImpactRule", + "pluginKey": "blah", + "pluginName": "Blah", + "languageKey": "xoo", + "languageName": "Xoo", + "cleanCodeAttributeCategory": "INTENTIONAL", + "left": { + "severity": "BLOCKER", + "impacts": [ + { + "softwareQuality": "SECURITY", + "severity": "MEDIUM" + } + ], + "params": {} + }, + "right": { + "severity": "BLOCKER", + "impacts": [ + { + "softwareQuality": "SECURITY", + "severity": "BLOCKER" + } + + ], + "params": {} + } + } + ], + "same": [] +} diff --git a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_impacts_left_only.json b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_impacts_left_only.json new file mode 100644 index 00000000000..bfd7c59419b --- /dev/null +++ b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_impacts_left_only.json @@ -0,0 +1,30 @@ +{ + "left": { + "key": "xoo-profile-1-01234", + "name": "Profile 1" + }, + "right": { + "key": "xoo-profile-2-12345", + "name": "Profile 2" + }, + "inLeft": [ + { + "key": "blah:impactRule", + "name": "ImpactRule", + "pluginKey": "blah", + "pluginName": "Blah", + "languageKey": "xoo", + "languageName": "Xoo", + "cleanCodeAttributeCategory": "INTENTIONAL", + "impacts": [ + { + "softwareQuality": "SECURITY", + "severity": "LOW" + } + ] + } + ], + "inRight": [], + "modified": [], + "same": [] +} diff --git a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_nominal.json b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_nominal.json index d2885aac763..3f5a23d3f8a 100644 --- a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_nominal.json +++ b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_nominal.json @@ -67,20 +67,16 @@ "languageName": "Xoo", "name" : "Rule4", "cleanCodeAttributeCategory": "INTENTIONAL", - "impacts": [ - { - "softwareQuality": "RELIABILITY", - "severity": "HIGH" - } - ], "left" : { "severity" : "BLOCKER", + "impacts": [], "params" : { "param_rule4" : "polop" } }, "right" : { "severity" : "BLOCKER", + "impacts": [], "params" : { "param_rule4" : "palap" } @@ -94,17 +90,42 @@ "languageName": "Xoo", "name" : "Rule5", "cleanCodeAttributeCategory": "INTENTIONAL", - "impacts": [ - { - "softwareQuality": "RELIABILITY", - "severity": "HIGH" - } - ], "left" : { - "severity" : "MINOR" + "severity" : "MINOR", + "impacts": [] }, "right" : { - "severity" : "MAJOR" + "severity" : "MAJOR", + "impacts": [] + } + }, + { + "key": "blah:rule6", + "name": "Rule6", + "pluginKey": "blah", + "pluginName": "Blah", + "languageKey": "xoo", + "languageName": "Xoo", + "cleanCodeAttributeCategory": "INTENTIONAL", + "left": { + "severity": "BLOCKER", + "impacts": [ + { + "softwareQuality": "SECURITY", + "severity": "BLOCKER" + } + ], + "params": {} + }, + "right": { + "severity": "BLOCKER", + "impacts": [ + { + "softwareQuality": "SECURITY", + "severity": "HIGH" + } + ], + "params": {} } } ] diff --git a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_left.json b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_left.json index afe92f1d381..26621a2567f 100644 --- a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_left.json +++ b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_left.json @@ -20,12 +20,14 @@ "name" : "Rule1", "left" : { "severity" : "BLOCKER", + "impacts": [], "params" : { "param_rule1" : "polop" } }, "right" : { - "severity" : "BLOCKER" + "severity" : "BLOCKER", + "impacts": [] } } ] diff --git a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_right.json b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_right.json index ec902279be3..b83e68c3463 100644 --- a/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_right.json +++ b/server/sonar-webserver-webapi/src/it/resources/org/sonar/server/qualityprofile/ws/CompareActionIT/compare_param_on_right.json @@ -19,10 +19,12 @@ "languageName": "Xoo", "name" : "Rule1", "left" : { - "severity" : "BLOCKER" + "severity" : "BLOCKER", + "impacts": [] }, "right" : { "severity" : "BLOCKER", + "impacts": [], "params" : { "param_rule1" : "polop" } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileComparison.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileComparison.java index ccb83d2a88f..42464c303e2 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileComparison.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileComparison.java @@ -21,26 +21,28 @@ package org.sonar.server.qualityprofile; import com.google.common.collect.MapDifference; import com.google.common.collect.Maps; -import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; 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.issue.ImpactDto; 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.rule.RuleDto; @ServerSide public class QProfileComparison { private final DbClient dbClient; - public QProfileComparison(DbClient dbClient) { this.dbClient = dbClient; } @@ -53,24 +55,34 @@ public class QProfileComparison { allRules.addAll(leftActiveRulesByRuleKey.keySet()); allRules.addAll(rightActiveRulesByRuleKey.keySet()); - QProfileComparisonResult result = new QProfileComparisonResult(left, right); + List referencedRules = dbClient.ruleDao().selectByKeys(dbSession, allRules); + Map rulesByKey = Maps.uniqueIndex(referencedRules, RuleDto::getKey); + + QProfileComparisonResult result = new QProfileComparisonResult(left, right, rulesByKey); for (RuleKey ruleKey : allRules) { if (!leftActiveRulesByRuleKey.containsKey(ruleKey)) { result.inRight.put(ruleKey, rightActiveRulesByRuleKey.get(ruleKey)); } else if (!rightActiveRulesByRuleKey.containsKey(ruleKey)) { result.inLeft.put(ruleKey, leftActiveRulesByRuleKey.get(ruleKey)); } else { - compareActivationParams(dbSession, leftActiveRulesByRuleKey.get(ruleKey), rightActiveRulesByRuleKey.get(ruleKey), result); + compareActiveRules(dbSession, + rulesByKey.get(ruleKey), + leftActiveRulesByRuleKey.get(ruleKey), + rightActiveRulesByRuleKey.get(ruleKey), + result); } } return result; } - private void compareActivationParams(DbSession session, ActiveRuleDto leftRule, ActiveRuleDto rightRule, QProfileComparisonResult result) { + private void compareActiveRules(DbSession session, RuleDto ruleDto, ActiveRuleDto leftRule, ActiveRuleDto rightRule, + QProfileComparisonResult result) { RuleKey key = leftRule.getRuleKey(); Map leftParams = paramDtoToMap(dbClient.activeRuleDao().selectParamsByActiveRuleUuid(session, leftRule.getUuid())); Map rightParams = paramDtoToMap(dbClient.activeRuleDao().selectParamsByActiveRuleUuid(session, rightRule.getUuid())); - if (leftParams.equals(rightParams) && leftRule.getSeverityString().equals(rightRule.getSeverityString())) { + if (leftParams.equals(rightParams) + && leftRule.getSeverityString().equals(rightRule.getSeverityString()) + && leftRule.getImpacts().equals(rightRule.getImpacts())) { result.same.put(key, leftRule); } else { ActiveRuleDiff diff = new ActiveRuleDiff(); @@ -78,17 +90,33 @@ public class QProfileComparison { diff.leftSeverity = leftRule.getSeverityString(); diff.rightSeverity = rightRule.getSeverityString(); + diff.impactDifference = Maps.difference( + computeEffectiveImpactMap(ruleDto, leftRule.getImpacts()), + computeEffectiveImpactMap(ruleDto, rightRule.getImpacts())); + diff.paramDifference = Maps.difference(leftParams, rightParams); result.modified.put(key, diff); } } + private static Map computeEffectiveImpactMap(RuleDto ruleDto, Map activeRuleImpacts) { + Map impacts = ruleDto.getDefaultImpactsMap(); + impacts.replaceAll(activeRuleImpacts::getOrDefault); + return impacts; + } + + public static List computeEffectiveImpacts(RuleDto ruleDto, Map activeRuleImpacts) { + Map impacts = computeEffectiveImpactMap(ruleDto, activeRuleImpacts); + return impacts.entrySet().stream().map(e -> new ImpactDto(e.getKey(), e.getValue())).toList(); + } + private Map loadActiveRules(DbSession dbSession, QProfileDto profile) { return Maps.uniqueIndex(dbClient.activeRuleDao().selectByProfile(dbSession, profile), ActiveRuleDto::getRuleKey); } public static class QProfileComparisonResult { + private final Map rulesByKey; private final QProfileDto left; private final QProfileDto right; private final Map inLeft = new HashMap<>(); @@ -96,9 +124,10 @@ public class QProfileComparison { private final Map modified = new HashMap<>(); private final Map same = new HashMap<>(); - public QProfileComparisonResult(QProfileDto left, QProfileDto right) { + public QProfileComparisonResult(QProfileDto left, QProfileDto right, Map rulesByKey) { this.left = left; this.right = right; + this.rulesByKey = rulesByKey; } public QProfileDto left() { @@ -125,19 +154,15 @@ public class QProfileComparison { return same; } - public Collection collectRuleKeys() { - Set keys = new HashSet<>(); - keys.addAll(inLeft.keySet()); - keys.addAll(inRight.keySet()); - keys.addAll(modified.keySet()); - keys.addAll(same.keySet()); - return keys; + public Map getImpactedRules() { + return rulesByKey; } } public static class ActiveRuleDiff { private String leftSeverity; private String rightSeverity; + private MapDifference impactDifference; private MapDifference paramDifference; public String leftSeverity() { @@ -148,6 +173,10 @@ public class QProfileComparison { return rightSeverity; } + public MapDifference impactDifference() { + return impactDifference; + } + public MapDifference paramDifference() { return paramDifference; } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/CompareAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/CompareAction.java index 8ccfd936515..62491257a64 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/CompareAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/CompareAction.java @@ -26,6 +26,9 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import javax.annotation.Nullable; +import org.jetbrains.annotations.NotNull; +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.resources.Language; import org.sonar.api.resources.Languages; import org.sonar.api.rule.RuleKey; @@ -96,7 +99,8 @@ public class CompareAction implements QProfileWsAction { .setChangelog( new Change("10.3", String.format("Added '%s' and '%s' fields", ATTRIBUTE_CLEAN_CODE_ATTRIBUTE_CATEGORY, ATTRIBUTE_IMPACTS)), new Change("10.3", String.format("Dropped '%s' field from '%s', '%s' and '%s' objects", - ATTRIBUTE_SEVERITY, ATTRIBUTE_SAME, ATTRIBUTE_IN_LEFT, ATTRIBUTE_IN_RIGHT))); + ATTRIBUTE_SEVERITY, ATTRIBUTE_SAME, ATTRIBUTE_IN_LEFT, ATTRIBUTE_IN_RIGHT)), + new Change("10.8", "'impacts' are part of the 'left' and 'right' sections of the 'modified' array")); compare.createParam(PARAM_LEFT_KEY) .setDescription("Profile key.") @@ -122,8 +126,7 @@ public class CompareAction implements QProfileWsAction { QProfileComparisonResult result = comparator.compare(dbSession, left, right); - List referencedRules = dbClient.ruleDao().selectByKeys(dbSession, new ArrayList<>(result.collectRuleKeys())); - Map rulesByKey = Maps.uniqueIndex(referencedRules, RuleDto::getKey); + Map rulesByKey = result.getImpactedRules(); Map repositoriesByKey = Maps.uniqueIndex(dbClient.ruleRepositoryDao().selectAll(dbSession), RuleRepositoryDto::getKey); writeResult(response.newJsonWriter(), result, rulesByKey, repositoriesByKey); } @@ -163,10 +166,11 @@ public class CompareAction implements QProfileWsAction { private void writeRules(JsonWriter json, Map activeRules, Map rulesByKey, Map repositoriesByKey) { json.beginArray(); - for (RuleKey key : activeRules.keySet()) { + for (ActiveRuleDto activeRuleDto : activeRules.values()) { json.beginObject(); - RuleDto rule = rulesByKey.get(key); + RuleDto rule = rulesByKey.get(activeRuleDto.getRuleKey()); writeRule(json, rule, repositoriesByKey.get(rule.getRepositoryKey())); + writeActiveRuleImpacts(json, rule, activeRuleDto); json.endObject(); } json.endArray(); @@ -191,15 +195,23 @@ public class CompareAction implements QProfileWsAction { if (cleanCodeAttribute != null) { json.prop(ATTRIBUTE_CLEAN_CODE_ATTRIBUTE_CATEGORY, cleanCodeAttribute.getAttributeCategory().toString()); } + } + + private static void writeActiveRuleImpacts(JsonWriter json, RuleDto rule, ActiveRuleDto activeRuleDto) { json.name(ATTRIBUTE_IMPACTS); json.beginArray(); - for (ImpactDto impact : rule.getDefaultImpacts()) { + List effectiveImpacts = QProfileComparison.computeEffectiveImpacts(rule, activeRuleDto.getImpacts()); + writeImpacts(json, effectiveImpacts); + json.endArray(); + } + + private static void writeImpacts(JsonWriter json, List effectiveImpacts) { + for (ImpactDto impact : effectiveImpacts) { json.beginObject(); json.prop(ATTRIBUTE_IMPACT_SOFTWARE_QUALITY, impact.getSoftwareQuality().toString()); json.prop(ATTRIBUTE_IMPACT_SEVERITY, impact.getSeverity().toString()); json.endObject(); } - json.endArray(); } private void writeDifferences(JsonWriter json, Map modified, Map rulesByKey, @@ -214,6 +226,14 @@ public class CompareAction implements QProfileWsAction { json.name(ATTRIBUTE_LEFT).beginObject(); json.prop(ATTRIBUTE_SEVERITY, value.leftSeverity()); + + + List leftImpacts = getLeftImpactDtos(value); + json.name(ATTRIBUTE_IMPACTS); + json.beginArray(); + writeImpacts(json, leftImpacts); + json.endArray(); + json.name(ATTRIBUTE_PARAMS).beginObject(); for (Entry> valueDiff : value.paramDifference().entriesDiffering().entrySet()) { json.prop(valueDiff.getKey(), valueDiff.getValue().leftValue()); @@ -228,6 +248,14 @@ public class CompareAction implements QProfileWsAction { json.name(ATTRIBUTE_RIGHT).beginObject(); json.prop(ATTRIBUTE_SEVERITY, value.rightSeverity()); + + List rightImpacts = getRightImpactDtos(value); + + json.name(ATTRIBUTE_IMPACTS); + json.beginArray(); + writeImpacts(json, rightImpacts); + json.endArray(); + json.name(ATTRIBUTE_PARAMS).beginObject(); for (Entry> valueDiff : value.paramDifference().entriesDiffering().entrySet()) { json.prop(valueDiff.getKey(), valueDiff.getValue().rightValue()); @@ -246,4 +274,26 @@ public class CompareAction implements QProfileWsAction { json.endArray(); } + private static @NotNull List getRightImpactDtos(ActiveRuleDiff activeRuleDiff) { + List rightImpacts = new ArrayList<>(); + for (Entry> valueDiff : activeRuleDiff.impactDifference().entriesDiffering().entrySet()) { + rightImpacts.add(new ImpactDto(valueDiff.getKey(), valueDiff.getValue().rightValue())); + } + for (Entry valueDiff : activeRuleDiff.impactDifference().entriesOnlyOnRight().entrySet()) { + rightImpacts.add(new ImpactDto(valueDiff.getKey(), valueDiff.getValue())); + } + return rightImpacts; + } + + private static @NotNull List getLeftImpactDtos(ActiveRuleDiff activeRuleDiff) { + List leftImpacts = new ArrayList<>(); + for (Entry> valueDiff : activeRuleDiff.impactDifference().entriesDiffering().entrySet()) { + leftImpacts.add(new ImpactDto(valueDiff.getKey(), valueDiff.getValue().leftValue())); + } + for (Entry valueDiff : activeRuleDiff.impactDifference().entriesOnlyOnLeft().entrySet()) { + leftImpacts.add(new ImpactDto(valueDiff.getKey(), valueDiff.getValue())); + } + return leftImpacts; + } + } diff --git a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/compare-example.json b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/compare-example.json index 18dcb2d7e64..41ef48e6174 100644 --- a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/compare-example.json +++ b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/compare-example.json @@ -67,20 +67,26 @@ "languageName": "JavaScript", "name" : "Avoid function with too many parameters", "cleanCodeAttributeCategory": "ADAPTABLE", - "impacts": [ - { - "softwareQuality": "MAINTAINABILITY", - "severity": "HIGH" - } - ], "right" : { "severity" : "MAJOR", + "impacts": [ + { + "softwareQuality": "MAINTAINABILITY", + "severity": "HIGH" + } + ], "params" : { "maximumFunctionParameters" : "7" } }, "left" : { "severity" : "MAJOR", + "impacts": [ + { + "softwareQuality": "MAINTAINABILITY", + "severity": "MEDIUM" + } + ], "params" : { "maximumFunctionParameters" : "10" }