From 5fdc55ea9f2d4678dc491058f27f12a3a5607525 Mon Sep 17 00:00:00 2001 From: =?utf8?q?L=C3=A9o=20Geoffroy?= Date: Thu, 7 Nov 2024 13:56:50 +0100 Subject: [PATCH] SONAR-23564 Update mapping logic for active rule impacts and severities --- .../rule/registration/RulesRegistrantIT.java | 24 +++++++++---------- .../builtin/RuleActivatorIT.java | 4 ++-- .../QProfileImpactSeverityMapper.java | 4 ---- .../QProfileImpactSeverityMapperTest.java | 8 +++---- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java b/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java index 06b55920c39..3e379453cc7 100644 --- a/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java +++ b/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java @@ -74,7 +74,6 @@ import org.sonar.server.es.EsTester; import org.sonar.server.es.SearchIdResult; import org.sonar.server.es.SearchOptions; import org.sonar.server.es.metadata.MetadataIndex; -import org.sonar.server.plugins.DetectPluginChange; import org.sonar.server.plugins.ServerPluginRepository; import org.sonar.server.property.InternalProperties; import org.sonar.server.property.MapInternalProperties; @@ -183,8 +182,7 @@ class RulesRegistrantIT { .key(s.getKey()) .content(s.getHtmlContent()) .context(s.getContext().map(c -> RuleDescriptionSectionContextDto.of(c.getKey(), c.getDisplayName())).orElse(null)) - .build() - ) + .build()) .collect(Collectors.toSet()); return Sets.union(ruleDescriptionSectionDtos, Set.of(builder().uuid(UuidFactoryFast.getInstance().create()).key("default").content(description).build())); }); @@ -439,7 +437,7 @@ class RulesRegistrantIT { assertThat(rule1.getType()).isEqualTo(RuleType.BUG.getDbConstant()); assertThat(rule1.getCreatedAt()).isEqualTo(DATE1.getTime()); assertThat(rule1.getUpdatedAt()).isEqualTo(DATE2.getTime()); - assertThat(rule1.getEducationPrinciples()).containsOnly("concept1","concept4"); + assertThat(rule1.getEducationPrinciples()).containsOnly("concept1", "concept4"); } @Test @@ -689,7 +687,7 @@ class RulesRegistrantIT { } private static Object[][] allRenamingCases() { - return new Object[][]{ + return new Object[][] { {"repo1", "rule1", "repo1", "rule2"}, {"repo1", "rule1", "repo2", "rule1"}, {"repo1", "rule1", "repo2", "rule2"}, @@ -772,7 +770,7 @@ class RulesRegistrantIT { RuleDescriptionSection section1context1 = createRuleDescriptionSection(HOW_TO_FIX_SECTION_KEY, "section1 ctx1 content", "ctx_1"); RuleDescriptionSection section1context2 = createRuleDescriptionSection(HOW_TO_FIX_SECTION_KEY, "section1 ctx2 content", "ctx_2"); RuleDescriptionSection section2context1 = createRuleDescriptionSection(RESOURCES_SECTION_KEY, "section2 content", "ctx_1"); - RuleDescriptionSection section2context2 = createRuleDescriptionSection(RESOURCES_SECTION_KEY,"section2 ctx2 content", "ctx_2"); + RuleDescriptionSection section2context2 = createRuleDescriptionSection(RESOURCES_SECTION_KEY, "section2 ctx2 content", "ctx_2"); RuleDescriptionSection section3noContext = createRuleDescriptionSection(ASSESS_THE_PROBLEM_SECTION_KEY, "section3 content", null); RuleDescriptionSection section4noContext = createRuleDescriptionSection(ROOT_CAUSE_SECTION_KEY, "section4 content", null); executeWithPluginRules(context -> { @@ -1133,15 +1131,15 @@ class RulesRegistrantIT { @Test void removed_rule_should_appear_in_changelog() { - //GIVEN + // GIVEN QProfileDto qProfileDto = db.qualityProfiles().insert(); RuleDto ruleDto = db.rules().insert(RULE_KEY1); db.qualityProfiles().activateRule(qProfileDto, ruleDto); ActiveRuleChange arChange = new ActiveRuleChange(DEACTIVATED, ActiveRuleDto.createFor(qProfileDto, ruleDto), ruleDto); when(qProfileRules.deleteRule(any(DbSession.class), eq(ruleDto))).thenReturn(List.of(arChange)); - //WHEN + // WHEN executeWithPluginRules(context -> context.createRepository("fake", "java").done()); - //THEN + // THEN List qProfileChangeDtos = dbClient.qProfileChangeDao().selectByQuery(db.getSession(), new QProfileChangeQuery(qProfileDto.getKee())); assertThat(qProfileChangeDtos).extracting(QProfileChangeDto::getRulesProfileUuid, QProfileChangeDto::getChangeType, QProfileChangeDto::getSqVersion) .contains(tuple(qProfileDto.getRulesProfileUuid(), "DEACTIVATED", sonarQubeVersion.toString())); @@ -1149,13 +1147,13 @@ class RulesRegistrantIT { @Test void removed_rule_should_be_deleted_when_renamed_repository() { - //GIVEN + // GIVEN RuleDto removedRuleDto = db.rules().insert(RuleKey.of("old_repo", "removed_rule")); RuleDto renamedRuleDto = db.rules().insert(RuleKey.of("old_repo", "renamed_rule")); - //WHEN + // WHEN executeWithPluginRules(context -> createRule(context, "java", "new_repo", renamedRuleDto.getRuleKey(), rule -> rule.addDeprecatedRuleKey(renamedRuleDto.getRepositoryKey(), renamedRuleDto.getRuleKey()))); - //THEN + // THEN verify(qProfileRules).deleteRule(any(DbSession.class), eq(removedRuleDto)); } @@ -1235,6 +1233,7 @@ class RulesRegistrantIT { repo.createRule("rule1") .setName("any") .setHtmlDescription("html") + .setType(RuleType.VULNERABILITY) .addDefaultImpact(SoftwareQuality.SECURITY, Severity.HIGH) .setSeverity(SeverityUtil.getSeverityFromOrdinal(2)); repo.done(); @@ -1256,6 +1255,7 @@ class RulesRegistrantIT { repo.createRule("rule1") .setName("any") .setHtmlDescription("html") + .setType(RuleType.VULNERABILITY) .addDefaultImpact(SoftwareQuality.SECURITY, Severity.LOW) .setSeverity(SeverityUtil.getSeverityFromOrdinal(2)); repo.done(); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/builtin/RuleActivatorIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/builtin/RuleActivatorIT.java index 3f385c9e398..294d99d938e 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/builtin/RuleActivatorIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/builtin/RuleActivatorIT.java @@ -174,7 +174,7 @@ class RuleActivatorIT { } @Test - void activate_whenOnlyOneImpactAndImpactDoesntMatchRuleType_shouldOverrideSeverity() { + void activate_whenOnlyOneImpactAndImpactDoesntMatchRuleType_shouldNotOverrideSeverity() { RuleDto rule = db.rules().insert(r -> r.setLanguage("xoo").setSeverity(Severity.BLOCKER) .replaceAllDefaultImpacts(List.of(newImpactDto(SECURITY, org.sonar.api.issue.impact.Severity.BLOCKER)))); RuleParamDto ruleParam = db.rules().insertRuleParam(rule, p -> p.setName("min").setDefaultValue("10")); @@ -206,7 +206,7 @@ class RuleActivatorIT { assertThat(result).hasSize(1); ActiveRuleChange activeRuleResult = result.get(0); - assertThat(activeRuleResult.getSeverity()).isEqualTo(Severity.MINOR); + assertThat(activeRuleResult.getSeverity()).isEqualTo(Severity.BLOCKER); assertThat(activeRuleResult.getNewImpacts()).containsExactlyEntriesOf(Map.of(SECURITY, org.sonar.api.issue.impact.Severity.LOW)); assertThat(activeRuleResult.getInheritance()).isEqualTo(OVERRIDES); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java index 5f570e02dd4..6e03441536d 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java @@ -46,8 +46,6 @@ public class QProfileImpactSeverityMapper { SoftwareQuality softwareQuality = ImpactMapper.convertToSoftwareQuality(ruleType); if (ruleImpacts.containsKey(softwareQuality)) { result.put(softwareQuality, ImpactSeverityMapper.mapImpactSeverity(severity)); - } else if (ruleImpacts.size() == 1) { - result.replaceAll((sq, sev) -> ImpactSeverityMapper.mapImpactSeverity(severity)); } return result; } @@ -57,8 +55,6 @@ public class QProfileImpactSeverityMapper { SoftwareQuality softwareQuality = ImpactMapper.convertToSoftwareQuality(ruleType); if (impacts.containsKey(softwareQuality)) { return ImpactSeverityMapper.mapRuleSeverity(impacts.get(softwareQuality)); - } else if (impacts.size() == 1) { - return ImpactSeverityMapper.mapRuleSeverity(impacts.entrySet().iterator().next().getValue()); } return ruleSeverity; } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapperTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapperTest.java index c38b07d31f2..82a334c3428 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapperTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapperTest.java @@ -64,13 +64,13 @@ class QProfileImpactSeverityMapperTest { } @Test - void mapImpactSeverities_whenOneDifferentImpact_shouldReturnOverriddenImpact() { + void mapImpactSeverities_whenOneDifferentImpact_shouldNotOverrideImpact() { Map result = QProfileImpactSeverityMapper.mapImpactSeverities(Severity.INFO, Map.of(SoftwareQuality.RELIABILITY, org.sonar.api.issue.impact.Severity.HIGH), RuleType.CODE_SMELL); - assertThat(result).hasSize(1).containsEntry(SoftwareQuality.RELIABILITY, org.sonar.api.issue.impact.Severity.INFO); + assertThat(result).hasSize(1).containsEntry(SoftwareQuality.RELIABILITY, org.sonar.api.issue.impact.Severity.HIGH); } @Test @@ -111,12 +111,12 @@ class QProfileImpactSeverityMapperTest { } @Test - void mapSeverity_whenOneImpact_ShouldReturnMappedImpactSeverity() { + void mapSeverity_whenOneImpactNotMatchingRuleType_ShouldReturnSameSeverity() { String severity = QProfileImpactSeverityMapper.mapSeverity( Map.of(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.HIGH), RuleType.BUG, Severity.BLOCKER); - assertThat(severity).isEqualTo(Severity.CRITICAL); + assertThat(severity).isEqualTo(Severity.BLOCKER); } @Test -- 2.39.5