]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-23258 fix bug with custom rules
authorLéo Geoffroy <leo.geoffroy@sonarsource.com>
Tue, 15 Oct 2024 12:14:59 +0000 (14:14 +0200)
committersonartech <sonartech@sonarsource.com>
Wed, 16 Oct 2024 20:03:02 +0000 (20:03 +0000)
server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java
server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java
server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileBackuperImplIT.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java

index a904093cf9840c2c5418c98066c2fcac2fed6bc8..dea97caa5287336ac2ec838e07c0e7287d5cfab4 100644 (file)
@@ -89,7 +89,8 @@ public class RuleCreatorIT {
   private final DbSession dbSession = dbTester.getSession();
   private final UuidFactory uuidFactory = new SequenceUuidFactory();
 
-  private final RuleCreator underTest = new RuleCreator(system2, new RuleIndexer(es.client(), dbTester.getDbClient()), dbTester.getDbClient(), newFullTypeValidations(), uuidFactory);
+  private final RuleCreator underTest = new RuleCreator(system2, new RuleIndexer(es.client(), dbTester.getDbClient()), dbTester.getDbClient(), newFullTypeValidations(),
+    uuidFactory);
 
   @Test
   public void create_custom_rule() {
@@ -357,6 +358,7 @@ public class RuleCreatorIT {
       .setName("My custom")
       .setMarkdownDescription("some description")
       .setSeverity(Severity.MAJOR)
+      .setImpacts(List.of(new NewCustomRule.Impact(RELIABILITY, MEDIUM)))
       .setStatus(RuleStatus.READY);
 
     NewCustomRule secondRule = NewCustomRule.createForCustomRule(RuleKey.parse("java:CUSTOM_RULE_2"), templateRule.getKey())
@@ -365,7 +367,7 @@ public class RuleCreatorIT {
       .setSeverity(Severity.MAJOR)
       .setStatus(RuleStatus.READY);
 
-    List<RuleKey> customRuleKeys = underTest.create(dbSession, Arrays.asList(firstRule, secondRule))
+    List<RuleKey> customRuleKeys = underTest.restore(dbSession, Arrays.asList(firstRule, secondRule))
       .stream()
       .map(RuleDto::getKey)
       .toList();
@@ -376,6 +378,11 @@ public class RuleCreatorIT {
     assertThat(rules).asList()
       .extracting("ruleKey")
       .containsOnly("CUSTOM_RULE_1", "CUSTOM_RULE_2");
+
+    RuleDto customRule1 = rules.stream().filter(e -> e.getRuleKey().equals("CUSTOM_RULE_1")).findFirst().orElseThrow();
+    assertThat(customRule1.getSeverityString()).isEqualTo(Severity.MAJOR);
+    assertThat(customRule1.getDefaultImpactsMap()).containsExactlyInAnyOrderEntriesOf(Map.of(RELIABILITY, MEDIUM));
+
   }
 
   @Test
@@ -391,8 +398,8 @@ public class RuleCreatorIT {
       .setSeverity(Severity.MAJOR)
       .setStatus(RuleStatus.READY)
       .setParameters(Map.of("regex", "a.*"));
-
-    assertThatThrownBy(() -> underTest.create(dbSession, singletonList(newRule)))
+    List<NewCustomRule> newRules = singletonList(newRule);
+    assertThatThrownBy(() -> underTest.restore(dbSession, newRules))
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("This rule is not a template rule: java:S001");
   }
@@ -413,7 +420,7 @@ public class RuleCreatorIT {
       .setStatus(RuleStatus.READY);
 
     List<NewCustomRule> newRules = singletonList(newRule);
-    assertThatThrownBy(() -> underTest.create(dbSession, newRules))
+    assertThatThrownBy(() -> underTest.restore(dbSession, newRules))
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("The template key doesn't exist: java:S001");
   }
index 4b5fdcc844819ffae292170a26492d03d2b57c4c..004ff3188d47bfdbc3ebb52465533809898d0099 100644 (file)
@@ -85,7 +85,7 @@ public class RuleCreator {
       .orElseThrow(() -> new IllegalArgumentException(format(TEMPLATE_KEY_NOT_EXIST_FORMAT, templateKey)));
     checkArgument(templateRule.isTemplate(), "This rule is not a template rule: %s", templateKey.toString());
     checkArgument(templateRule.getStatus() != RuleStatus.REMOVED, TEMPLATE_KEY_NOT_EXIST_FORMAT, templateKey.toString());
-    validateCustomRule(newRule, dbSession, templateKey);
+    validateCustomRule(newRule, dbSession, templateKey, true);
 
     Optional<RuleDto> definition = loadRule(dbSession, newRule.ruleKey());
     RuleDto ruleDto = definition.map(dto -> updateExistingRule(dto, newRule, dbSession))
@@ -95,7 +95,7 @@ public class RuleCreator {
     return ruleDto;
   }
 
-  public List<RuleDto> create(DbSession dbSession, List<NewCustomRule> newRules) {
+  public List<RuleDto> restore(DbSession dbSession, List<NewCustomRule> newRules) {
     Set<RuleKey> templateKeys = newRules.stream().map(NewCustomRule::templateKey).collect(Collectors.toSet());
     Map<RuleKey, RuleDto> templateRules = dbClient.ruleDao().selectByKeys(dbSession, templateKeys)
       .stream()
@@ -112,7 +112,7 @@ public class RuleCreator {
     List<RuleDto> customRules = newRules.stream()
       .map(newCustomRule -> {
         RuleDto templateRule = templateRules.get(newCustomRule.templateKey());
-        validateCustomRule(newCustomRule, dbSession, templateRule.getKey());
+        validateCustomRule(newCustomRule, dbSession, templateRule.getKey(), false);
         return createCustomRule(newCustomRule, templateRule, dbSession);
       })
       .toList();
@@ -121,7 +121,7 @@ public class RuleCreator {
     return customRules;
   }
 
-  private void validateCustomRule(NewCustomRule newRule, DbSession dbSession, RuleKey templateKey) {
+  private void validateCustomRule(NewCustomRule newRule, DbSession dbSession, RuleKey templateKey, boolean checkImpacts) {
     List<String> errors = new ArrayList<>();
 
     validateRuleKey(errors, newRule.ruleKey(), templateKey);
@@ -135,7 +135,7 @@ public class RuleCreator {
     if (severity != null && !Severity.ALL.contains(severity)) {
       errors.add(format("Severity \"%s\" is invalid", severity));
     }
-    if (!CollectionUtils.isEmpty(newRule.getImpacts()) && (StringUtils.isNotBlank(newRule.severity()) || newRule.type() != null)) {
+    if (checkImpacts && !CollectionUtils.isEmpty(newRule.getImpacts()) && (StringUtils.isNotBlank(newRule.severity()) || newRule.type() != null)) {
       errors.add("The rule cannot have both impacts and type/severity specified");
     }
 
index 286ca058acba8e06d447372608426ec572ad22d9..4204e3d1df022e594a03d70b8491af19f1a11a97 100644 (file)
@@ -425,14 +425,12 @@ class QProfileBackuperImplIT {
       Arguments.of(true, true, true),
       Arguments.of(false, false, false),
       Arguments.of(false, null, false),
-      Arguments.of(false, true, true)
-    );
+      Arguments.of(false, true, true));
   }
 
-
   @Test
   void restore_custom_rule() {
-    when(ruleCreator.create(any(), anyList())).then(invocation -> Collections.singletonList(db.rules().insert(RuleKey.of("sonarjs", "s001"))));
+    when(ruleCreator.restore(any(), anyList())).then(invocation -> Collections.singletonList(db.rules().insert(RuleKey.of("sonarjs", "s001"))));
 
     Reader backup = new StringReader("<?xml version='1.0' encoding='UTF-8'?>" +
       "<profile>" +
@@ -494,7 +492,7 @@ class QProfileBackuperImplIT {
   }
 
   @Test
-  void restore_should_override_impacts(){
+  void restore_should_override_impacts() {
     String ruleUuid = db.rules().insert(RuleKey.of("sonarjs", "s001")).getUuid();
 
     Reader backup = new StringReader("<?xml version='1.0' encoding='UTF-8'?>" +
index e6f8b9b78e48d349f8744c277cee90e4c2171df5..27345a955274bd4b3a4ff2e086b7d679117058a6 100644 (file)
@@ -204,7 +204,7 @@ public class QProfileBackuperImpl implements QProfileBackuper {
       .toList();
 
     if (!customRulesToCreate.isEmpty()) {
-      return db.ruleDao().selectByKeys(dbSession, ruleCreator.create(dbSession, customRulesToCreate).stream().map(RuleDto::getKey).toList())
+      return db.ruleDao().selectByKeys(dbSession, ruleCreator.restore(dbSession, customRulesToCreate).stream().map(RuleDto::getKey).toList())
         .stream()
         .collect(Collectors.toMap(RuleDto::getKey, identity()));
     }