]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-23564 Update mapping logic for active rule impacts and severities
authorLéo Geoffroy <leo.geoffroy@sonarsource.com>
Thu, 7 Nov 2024 12:56:50 +0000 (13:56 +0100)
committersonartech <sonartech@sonarsource.com>
Mon, 11 Nov 2024 20:02:44 +0000 (20:02 +0000)
server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java
server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/builtin/RuleActivatorIT.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapperTest.java

index 06b55920c390ec4bfad09c2a692dab0de90a925a..3e379453cc74b16c6e17441b05d83a63b75f0ea9 100644 (file)
@@ -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<QProfileChangeDto> 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();
index 3f385c9e398000a3acc6a214e3438bc2f2ed0ad9..294d99d938e005bec4408f93bb7d285ddef346fa 100644 (file)
@@ -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);
   }
index 5f570e02dd491f76fd918dfe8eee49f3c6d60670..6e03441536d722822b5d450d0573277ee0f20119 100644 (file)
@@ -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;
   }
index c38b07d31f2ec1226807bf7876293f73fadbabfd..82a334c34280584c2db2662e9726df81ce9cbbad 100644 (file)
@@ -64,13 +64,13 @@ class QProfileImpactSeverityMapperTest {
   }
 
   @Test
-  void mapImpactSeverities_whenOneDifferentImpact_shouldReturnOverriddenImpact() {
+  void mapImpactSeverities_whenOneDifferentImpact_shouldNotOverrideImpact() {
     Map<SoftwareQuality, org.sonar.api.issue.impact.Severity> 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