From aff39a4a01010cd4e6425ecd6f17146dd3274f89 Mon Sep 17 00:00:00 2001 From: =?utf8?q?L=C3=A9o=20Geoffroy?= Date: Tue, 15 Oct 2024 14:14:59 +0200 Subject: [PATCH] SONAR-23258 fix bug with custom rules --- .../sonar/server/common/rule/RuleCreatorIT.java | 17 ++++++++++++----- .../sonar/server/common/rule/RuleCreator.java | 10 +++++----- .../qualityprofile/QProfileBackuperImplIT.java | 8 +++----- .../qualityprofile/QProfileBackuperImpl.java | 2 +- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java index a904093cf98..dea97caa528 100644 --- a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java +++ b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java @@ -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 customRuleKeys = underTest.create(dbSession, Arrays.asList(firstRule, secondRule)) + List 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 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 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"); } diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java index 4b5fdcc8448..004ff3188d4 100644 --- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java +++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java @@ -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 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 create(DbSession dbSession, List newRules) { + public List restore(DbSession dbSession, List newRules) { Set templateKeys = newRules.stream().map(NewCustomRule::templateKey).collect(Collectors.toSet()); Map templateRules = dbClient.ruleDao().selectByKeys(dbSession, templateKeys) .stream() @@ -112,7 +112,7 @@ public class RuleCreator { List 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 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"); } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileBackuperImplIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileBackuperImplIT.java index 286ca058acb..4204e3d1df0 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileBackuperImplIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualityprofile/QProfileBackuperImplIT.java @@ -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("" + "" + @@ -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("" + diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java index e6f8b9b78e4..27345a95527 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java @@ -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())); } -- 2.39.5