From f4684e8e353a68e62a076f1b79593d33ef101c86 Mon Sep 17 00:00:00 2001 From: lukasz-jarocki-sonarsource Date: Fri, 11 Aug 2023 10:51:33 +0200 Subject: [PATCH] SONAR-20021 adding default impacts when registering a rule --- gradle.properties | 2 +- .../main/java/org/sonar/db/rule/RuleDao.java | 2 +- .../main/java/org/sonar/db/rule/RuleDto.java | 50 +------- .../java/org/sonar/db/rule/RuleDtoTest.java | 42 ++----- .../rule/registration/NewRuleCreator.java | 106 +++++++++++++++++ .../rule/registration/RulesRegistrant.java | 35 ++---- .../rule/registration/StartupRuleUpdater.java | 35 ++++-- .../rule/registration/NewRuleCreatorTest.java | 110 ++++++++++++++++++ .../rule/registration/RulesRegistrantIT.java | 25 ++-- .../platformlevel/PlatformLevelStartup.java | 2 + 10 files changed, 287 insertions(+), 122 deletions(-) create mode 100644 server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java create mode 100644 server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java diff --git a/gradle.properties b/gradle.properties index 232f3cfaac2..2bfc84a75a4 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,6 +1,6 @@ group=org.sonarsource.sonarqube version=10.2 -pluginApiVersion=10.0.0.790 +pluginApiVersion=10.1.0.809 description=Open source platform for continuous inspection of code quality projectTitle=SonarQube org.gradle.jvmargs=-Xmx2048m diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java index 1af002cc859..1edf6b812f3 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java @@ -129,7 +129,7 @@ public class RuleDao implements Dao { private static void insertRuleDefaultImpacts(RuleDto ruleDto, RuleMapper mapper) { ruleDto.getDefaultImpacts() - .forEach(section -> mapper.insertRuleDefaultImpact(ruleDto.getUuid(), section)); + .forEach(impact -> mapper.insertRuleDefaultImpact(ruleDto.getUuid(), impact)); } public void scrollIndexingRuleExtensionsByIds(DbSession dbSession, Collection ruleExtensionIds, Consumer consumer) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java index f72417e4a62..76940f89c4b 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java @@ -37,8 +37,6 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; -import org.sonar.api.server.debt.DebtRemediationFunction; -import org.sonar.api.server.rule.RulesDefinition; import org.sonar.db.issue.ImpactDto; import static com.google.common.base.Preconditions.checkArgument; @@ -46,7 +44,6 @@ import static java.lang.String.format; import static java.util.Arrays.asList; import static java.util.Collections.emptySet; import static java.util.Optional.ofNullable; -import static org.apache.commons.lang.StringUtils.isNotEmpty; import static org.sonar.db.rule.RuleDescriptionSectionDto.DEFAULT_KEY; public class RuleDto { @@ -420,6 +417,10 @@ public class RuleDto { return type; } + public RuleType getEnumType() { + return RuleType.valueOf(type); + } + public RuleDto setType(int type) { this.type = type; return this; @@ -655,49 +656,6 @@ public class RuleDto { return strings == null || strings.isEmpty() ? null : String.join(",", strings); } - public static RuleDto from(RulesDefinition.Rule ruleDef, Set ruleDescriptionSectionDtos, String uuid, - long now) { - RuleDto ruleDto = new RuleDto() - .setUuid(uuid) - .setRuleKey(RuleKey.of(ruleDef.repository().key(), ruleDef.key())) - .setPluginKey(ruleDef.pluginKey()) - .setIsTemplate(ruleDef.template()) - .setConfigKey(ruleDef.internalKey()) - .setLanguage(ruleDef.repository().language()) - .setName(ruleDef.name()) - .setSeverity(ruleDef.severity()) - .setStatus(ruleDef.status()) - .setGapDescription(ruleDef.gapDescription()) - .setSystemTags(ruleDef.tags()) - .setSecurityStandards(ruleDef.securityStandards()) - .setType(RuleType.valueOf(ruleDef.type().name())) - .setScope(Scope.valueOf(ruleDef.scope().name())) - .setIsExternal(ruleDef.repository().isExternal()) - .setIsAdHoc(false) - .setCreatedAt(now) - .setUpdatedAt(now) - .setCleanCodeAttribute(ruleDef.cleanCodeAttribute() != null ? ruleDef.cleanCodeAttribute() : CleanCodeAttribute.defaultCleanCodeAttribute()) - .setEducationPrinciples(ruleDef.educationPrincipleKeys()); - - if (isNotEmpty(ruleDef.htmlDescription())) { - ruleDto.setDescriptionFormat(Format.HTML); - } else if (isNotEmpty(ruleDef.markdownDescription())) { - ruleDto.setDescriptionFormat(Format.MARKDOWN); - } - - ruleDescriptionSectionDtos.forEach(ruleDto::addRuleDescriptionSectionDto); - - DebtRemediationFunction debtRemediationFunction = ruleDef.debtRemediationFunction(); - if (debtRemediationFunction != null) { - ruleDto.setDefRemediationFunction(debtRemediationFunction.type().name()); - ruleDto.setDefRemediationGapMultiplier(debtRemediationFunction.gapMultiplier()); - ruleDto.setDefRemediationBaseEffort(debtRemediationFunction.baseEffort()); - ruleDto.setGapDescription(ruleDef.gapDescription()); - } - - return ruleDto; - } - @Override public boolean equals(Object obj) { if (!(obj instanceof RuleDto)) { diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDtoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDtoTest.java index 58e8c7b1df5..d0ee6dcd3dc 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDtoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDtoTest.java @@ -175,24 +175,6 @@ public class RuleDtoTest { .hasMessage(String.format(ERROR_MESSAGE_SECTION_ALREADY_EXISTS, SECTION_KEY, contextKey)); } - @Test - public void from_whenRuleDefinitionDoesntHaveCleanCodeAttribute_shouldAlwaysSetCleanCodeAttribute() { - RulesDefinition.Rule ruleDef = getDefaultRule(); - - RuleDto newRuleDto = RuleDto.from(ruleDef, Set.of(), "uuid", Long.MAX_VALUE); - - assertThat(newRuleDto.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.CONVENTIONAL); - } - - @Test - public void from_whenRuleDefinitionDoesHaveCleanCodeAttribute_shouldReturnThisAttribute() { - RulesDefinition.Rule ruleDef = getDefaultRule(CleanCodeAttribute.TESTED); - - RuleDto newRuleDto = RuleDto.from(ruleDef, Set.of(), "uuid", Long.MAX_VALUE); - - assertThat(newRuleDto.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.TESTED); - } - @Test public void addDefaultImpact_whenSoftwareQualityAlreadyDefined_shouldThrowISE() { RuleDto dto = new RuleDto(); @@ -238,7 +220,16 @@ public class RuleDtoTest { .containsExactlyInAnyOrder( tuple(SoftwareQuality.MAINTAINABILITY, Severity.HIGH), tuple(SoftwareQuality.SECURITY, Severity.LOW)); + } + + @Test + public void getEnumType_shouldReturnCorrectValue() { + RuleDto ruleDto = new RuleDto(); + ruleDto.setType(RuleType.CODE_SMELL); + + RuleType enumType = ruleDto.getEnumType(); + assertThat(enumType).isEqualTo(RuleType.CODE_SMELL); } @NotNull @@ -262,20 +253,5 @@ public class RuleDtoTest { .setSoftwareQuality(softwareQuality) .setSeverity(severity); } - private static RulesDefinition.Rule getDefaultRule(@Nullable CleanCodeAttribute attribute) { - RulesDefinition.Rule ruleDef = mock(RulesDefinition.Rule.class); - RulesDefinition.Repository repository = mock(RulesDefinition.Repository.class); - when(ruleDef.repository()).thenReturn(repository); - - when(ruleDef.key()).thenReturn("key"); - when(repository.key()).thenReturn("repoKey"); - when(ruleDef.type()).thenReturn(RuleType.CODE_SMELL); - when(ruleDef.scope()).thenReturn(RuleScope.TEST); - when(ruleDef.cleanCodeAttribute()).thenReturn(attribute); - return ruleDef; - } - private static RulesDefinition.Rule getDefaultRule() { - return getDefaultRule(null); - } } diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java new file mode 100644 index 00000000000..5160840ba1f --- /dev/null +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java @@ -0,0 +1,106 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.rule.registration; + +import java.util.stream.Collectors; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.CleanCodeAttribute; +import org.sonar.api.rules.RuleType; +import org.sonar.api.server.debt.DebtRemediationFunction; +import org.sonar.api.server.rule.RulesDefinition; +import org.sonar.api.utils.System2; +import org.sonar.core.util.UuidFactory; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.issue.ImpactDto; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.rule.RuleDescriptionSectionsGeneratorResolver; + +import static org.apache.commons.lang.StringUtils.isNotEmpty; + +public class NewRuleCreator { + + private final DbClient dbClient; + private final RuleDescriptionSectionsGeneratorResolver ruleDescriptionSectionsGeneratorResolver; + private final UuidFactory uuidFactory; + private final System2 system2; + + public NewRuleCreator(DbClient dbClient, RuleDescriptionSectionsGeneratorResolver ruleDescriptionSectionsGeneratorResolver, UuidFactory uuidFactory, System2 system2) { + this.dbClient = dbClient; + this.ruleDescriptionSectionsGeneratorResolver = ruleDescriptionSectionsGeneratorResolver; + this.uuidFactory = uuidFactory; + this.system2 = system2; + } + + RuleDto createNewRule(RulesRegistrationContext context, RulesDefinition.Rule ruleDef, DbSession session) { + RuleDto newRule = createRuleWithSimpleFields(ruleDef, uuidFactory.create(), system2.now()); + + newRule.replaceAllDefaultImpacts(ruleDef.defaultImpacts().entrySet() + .stream() + .map(e -> new ImpactDto().setUuid(uuidFactory.create()).setSoftwareQuality(e.getKey()).setSeverity(e.getValue())) + .collect(Collectors.toSet())); + + ruleDescriptionSectionsGeneratorResolver.generateFor(ruleDef).forEach(newRule::addRuleDescriptionSectionDto); + + dbClient.ruleDao().insert(session, newRule); + context.created(newRule); + return newRule; + } + + RuleDto createRuleWithSimpleFields(RulesDefinition.Rule ruleDef, String uuid, long now) { + RuleDto ruleDto = new RuleDto() + .setUuid(uuid) + .setRuleKey(RuleKey.of(ruleDef.repository().key(), ruleDef.key())) + .setPluginKey(ruleDef.pluginKey()) + .setIsTemplate(ruleDef.template()) + .setConfigKey(ruleDef.internalKey()) + .setLanguage(ruleDef.repository().language()) + .setName(ruleDef.name()) + .setSeverity(ruleDef.severity()) + .setStatus(ruleDef.status()) + .setGapDescription(ruleDef.gapDescription()) + .setSystemTags(ruleDef.tags()) + .setSecurityStandards(ruleDef.securityStandards()) + .setType(RuleType.valueOf(ruleDef.type().name())) + .setScope(RuleDto.Scope.valueOf(ruleDef.scope().name())) + .setIsExternal(ruleDef.repository().isExternal()) + .setIsAdHoc(false) + .setCreatedAt(now) + .setUpdatedAt(now) + .setCleanCodeAttribute(ruleDef.cleanCodeAttribute() != null ? ruleDef.cleanCodeAttribute() : CleanCodeAttribute.defaultCleanCodeAttribute()) + .setEducationPrinciples(ruleDef.educationPrincipleKeys()); + + if (isNotEmpty(ruleDef.htmlDescription())) { + ruleDto.setDescriptionFormat(RuleDto.Format.HTML); + } else if (isNotEmpty(ruleDef.markdownDescription())) { + ruleDto.setDescriptionFormat(RuleDto.Format.MARKDOWN); + } + + DebtRemediationFunction debtRemediationFunction = ruleDef.debtRemediationFunction(); + if (debtRemediationFunction != null) { + ruleDto.setDefRemediationFunction(debtRemediationFunction.type().name()); + ruleDto.setDefRemediationGapMultiplier(debtRemediationFunction.gapMultiplier()); + ruleDto.setDefRemediationBaseEffort(debtRemediationFunction.baseEffort()); + ruleDto.setGapDescription(ruleDef.gapDescription()); + } + + return ruleDto; + } +} diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java index cf69f4026a7..311fa04f82a 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java @@ -29,7 +29,6 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; -import javax.annotation.Nonnull; import org.sonar.api.Startable; import org.sonar.api.resources.Languages; import org.sonar.api.rule.RuleKey; @@ -39,7 +38,6 @@ import org.sonar.api.utils.System2; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Profiler; -import org.sonar.core.util.UuidFactory; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.rule.RuleDto; @@ -49,7 +47,6 @@ import org.sonar.server.qualityprofile.ActiveRuleChange; import org.sonar.server.qualityprofile.QProfileRules; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.RuleDefinitionsLoader; -import org.sonar.server.rule.RuleDescriptionSectionsGeneratorResolver; import org.sonar.server.rule.WebServerRuleFinder; import org.sonar.server.rule.index.RuleIndexer; @@ -73,17 +70,15 @@ public class RulesRegistrant implements Startable { private final Languages languages; private final System2 system2; private final WebServerRuleFinder webServerRuleFinder; - private final UuidFactory uuidFactory; private final MetadataIndex metadataIndex; - private final RuleDescriptionSectionsGeneratorResolver ruleDescriptionSectionsGeneratorResolver; private final RulesKeyVerifier rulesKeyVerifier; private final StartupRuleUpdater startupRuleUpdater; + private final NewRuleCreator newRuleCreator; public RulesRegistrant(RuleDefinitionsLoader defLoader, QProfileRules qProfileRules, DbClient dbClient, RuleIndexer ruleIndexer, - ActiveRuleIndexer activeRuleIndexer, Languages languages, System2 system2, - WebServerRuleFinder webServerRuleFinder, UuidFactory uuidFactory, MetadataIndex metadataIndex, - RuleDescriptionSectionsGeneratorResolver ruleDescriptionSectionsGeneratorResolver, - RulesKeyVerifier rulesKeyVerifier, StartupRuleUpdater startupRuleUpdater) { + ActiveRuleIndexer activeRuleIndexer, Languages languages, System2 system2, WebServerRuleFinder webServerRuleFinder, + MetadataIndex metadataIndex, RulesKeyVerifier rulesKeyVerifier, StartupRuleUpdater startupRuleUpdater, + NewRuleCreator newRuleCreator) { this.defLoader = defLoader; this.qProfileRules = qProfileRules; this.dbClient = dbClient; @@ -92,11 +87,10 @@ public class RulesRegistrant implements Startable { this.languages = languages; this.system2 = system2; this.webServerRuleFinder = webServerRuleFinder; - this.uuidFactory = uuidFactory; this.metadataIndex = metadataIndex; - this.ruleDescriptionSectionsGeneratorResolver = ruleDescriptionSectionsGeneratorResolver; this.rulesKeyVerifier = rulesKeyVerifier; this.startupRuleUpdater = startupRuleUpdater; + this.newRuleCreator = newRuleCreator; } @Override @@ -159,7 +153,7 @@ public class RulesRegistrant implements Startable { for (RulesDefinition.Rule ruleDef : ruleDefs) { RuleKey ruleKey = RuleKey.of(ruleDef.repository().key(), ruleDef.key()); - RuleDto ruleDto = findOrCreateRuleDto(context, session, ruleDef); + RuleDto ruleDto = context.getDbRuleFor(ruleDef).orElseGet(() -> newRuleCreator.createNewRule(context, ruleDef, session)); dtos.put(ruleDef, ruleDto); // we must detect renaming __before__ we modify the DTO @@ -168,7 +162,7 @@ public class RulesRegistrant implements Startable { ruleDto.setRuleKey(ruleKey); } - if (startupRuleUpdater.findChangesAndUpdateRule(ruleDef, ruleDto)) { + if (!context.isCreated(ruleDto) && startupRuleUpdater.findChangesAndUpdateRule(ruleDef, ruleDto)) { context.updated(ruleDto); } @@ -185,17 +179,6 @@ public class RulesRegistrant implements Startable { } } - @Nonnull - private RuleDto findOrCreateRuleDto(RulesRegistrationContext context, DbSession session, RulesDefinition.Rule ruleDef) { - return context.getDbRuleFor(ruleDef) - .orElseGet(() -> { - RuleDto newRule = RuleDto.from(ruleDef, ruleDescriptionSectionsGeneratorResolver.generateFor(ruleDef), uuidFactory.create(), system2.now()); - dbClient.ruleDao().insert(session, newRule); - context.created(newRule); - return newRule; - }); - } - private void processRemainingDbRules(RulesRegistrationContext recorder, DbSession dbSession) { // custom rules check status of template, so they must be processed at the end List customRules = new ArrayList<>(); @@ -314,8 +297,8 @@ public class RulesRegistrant implements Startable { private static Set getExistingAndRenamedRepositories(RulesRegistrationContext recorder, Collection context) { return Stream.concat( - context.stream().map(RulesDefinition.ExtendedRepository::key), - recorder.getRenamed().map(Map.Entry::getValue).map(RuleKey::repository)) + context.stream().map(RulesDefinition.ExtendedRepository::key), + recorder.getRenamed().map(Map.Entry::getValue).map(RuleKey::repository)) .collect(Collectors.toSet()); } diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java index 4bc1601452e..6b13dc3b76d 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java @@ -25,8 +25,11 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; @@ -39,6 +42,7 @@ import org.sonar.api.utils.log.Profiler; import org.sonar.core.util.UuidFactory; 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.rule.DeprecatedRuleKeyDto; @@ -153,7 +157,7 @@ public class StartupRuleUpdater { changed = true; } changed |= mergeCleanCodeAttribute(def, dto); - changed |= mergeImpacts(def, dto); + changed |= mergeImpacts(def, dto, uuidFactory); if (dto.isAdHoc()) { dto.setIsAdHoc(false); changed = true; @@ -161,12 +165,6 @@ public class StartupRuleUpdater { return changed; } - private static boolean mergeImpacts(RulesDefinition.Rule def, RuleDto dto) { - boolean changed = false; - // TODO when DTOs for impacts are ready - return changed; - } - private static boolean mergeCleanCodeAttribute(RulesDefinition.Rule def, RuleDto dto) { boolean changed = false; if (!Objects.equals(dto.getCleanCodeAttribute(), def.cleanCodeAttribute()) && (def.cleanCodeAttribute() != null)) { @@ -181,6 +179,29 @@ public class StartupRuleUpdater { return changed; } + boolean mergeImpacts(RulesDefinition.Rule def, RuleDto dto, UuidFactory uuidFactory) { + if (dto.getEnumType() == RuleType.SECURITY_HOTSPOT) { + return false; + } + + Map impactsFromPlugin = def.defaultImpacts(); + Map impactsFromDb = dto.getDefaultImpacts().stream().collect(Collectors.toMap(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity)); + + if (impactsFromPlugin.isEmpty()) { + throw new IllegalStateException("There should be at least one impact defined for the rule " + def.key()); + } + + if (!Objects.equals(impactsFromDb, impactsFromPlugin)) { + dto.replaceAllDefaultImpacts(impactsFromPlugin.entrySet() + .stream() + .map(e -> new ImpactDto().setUuid(uuidFactory.create()).setSoftwareQuality(e.getKey()).setSeverity(e.getValue())) + .collect(Collectors.toSet())); + return true; + } + + return false; + } + private static boolean mergeEducationPrinciples(RulesDefinition.Rule ruleDef, RuleDto dto) { boolean changed = false; if (dto.getEducationPrinciples().size() != ruleDef.educationPrincipleKeys().size() || diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java new file mode 100644 index 00000000000..d16a6f9122c --- /dev/null +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java @@ -0,0 +1,110 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.rule.registration; + +import java.util.HashMap; +import java.util.Map; +import org.jetbrains.annotations.Nullable; +import org.junit.Before; +import org.junit.Test; +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; +import org.sonar.api.rule.RuleScope; +import org.sonar.api.rules.CleanCodeAttribute; +import org.sonar.api.rules.RuleType; +import org.sonar.api.server.rule.RulesDefinition; +import org.sonar.api.utils.System2; +import org.sonar.core.util.UuidFactory; +import org.sonar.db.DbClient; +import org.sonar.db.issue.ImpactDto; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.rule.RuleDescriptionSectionsGeneratorResolver; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.groups.Tuple.tuple; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.sonar.api.rule.Severity.MAJOR; + +public class NewRuleCreatorTest { + + private final DbClient dbClient = mock(); + private final RuleDescriptionSectionsGeneratorResolver ruleDescriptionSectionsGeneratorResolver = mock(); + private final UuidFactory uuidFactory = mock(); + private final System2 system2 = mock(); + private final RulesRegistrationContext context = mock(); + + private final NewRuleCreator underTest = new NewRuleCreator(dbClient, ruleDescriptionSectionsGeneratorResolver, uuidFactory, system2); + + @Before + public void before() { + when(dbClient.ruleDao()).thenReturn(mock()); + } + + @Test + public void from_whenRuleDefinitionDoesntHaveCleanCodeAttribute_shouldAlwaysSetCleanCodeAttribute() { + RulesDefinition.Rule ruleDef = getDefaultRule(); + + RuleDto newRuleDto = underTest.createNewRule(context, ruleDef, mock()); + + assertThat(newRuleDto.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.CONVENTIONAL); + } + + @Test + public void from_whenRuleDefinitionDoesHaveCleanCodeAttribute_shouldReturnThisAttribute() { + RulesDefinition.Rule ruleDef = getDefaultRule(CleanCodeAttribute.TESTED); + + RuleDto newRuleDto = underTest.createNewRule(context, ruleDef, mock()); + + assertThat(newRuleDto.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.TESTED); + } + + @Test + public void createNewRule_whenImpactDefined_shouldReturnThisImpact() { + RulesDefinition.Rule ruleDef = getDefaultRule(); + Map singleImpact = new HashMap<>(); + singleImpact.put(SoftwareQuality.RELIABILITY, Severity.LOW); + when(ruleDef.defaultImpacts()).thenReturn(singleImpact); + + RuleDto newRuleDto = underTest.createNewRule(context, ruleDef, mock()); + + assertThat(newRuleDto.getDefaultImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) + .containsOnly(tuple(SoftwareQuality.RELIABILITY, Severity.LOW)); + } + + private static RulesDefinition.Rule getDefaultRule(@Nullable CleanCodeAttribute attribute) { + RulesDefinition.Rule ruleDef = mock(RulesDefinition.Rule.class); + RulesDefinition.Repository repository = mock(RulesDefinition.Repository.class); + when(ruleDef.repository()).thenReturn(repository); + + when(ruleDef.key()).thenReturn("key"); + when(repository.key()).thenReturn("repoKey"); + when(ruleDef.type()).thenReturn(RuleType.CODE_SMELL); + when(ruleDef.scope()).thenReturn(RuleScope.TEST); + when(ruleDef.cleanCodeAttribute()).thenReturn(attribute); + when(ruleDef.severity()).thenReturn(MAJOR); + when(ruleDef.defaultImpacts()).thenReturn(Map.of(SoftwareQuality.MAINTAINABILITY, Severity.LOW)); + return ruleDef; + } + + private static RulesDefinition.Rule getDefaultRule() { + return getDefaultRule(null); + } +} diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java index a8cab706e40..9a936b48888 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java @@ -37,6 +37,8 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.sonar.api.impl.utils.TestSystem2; +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; @@ -55,6 +57,7 @@ import org.sonar.core.util.UuidFactoryFast; 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.ActiveRuleDto; import org.sonar.db.qualityprofile.QProfileChangeDto; import org.sonar.db.qualityprofile.QProfileChangeQuery; @@ -102,6 +105,7 @@ import static org.mockito.Mockito.when; import static org.sonar.api.rule.RuleStatus.READY; import static org.sonar.api.rule.RuleStatus.REMOVED; import static org.sonar.api.rule.Severity.BLOCKER; +import static org.sonar.api.rule.Severity.CRITICAL; import static org.sonar.api.rule.Severity.INFO; import static org.sonar.api.server.rule.RuleDescriptionSection.RuleDescriptionSectionKeys.ASSESS_THE_PROBLEM_SECTION_KEY; import static org.sonar.api.server.rule.RuleDescriptionSection.RuleDescriptionSectionKeys.HOW_TO_FIX_SECTION_KEY; @@ -155,6 +159,7 @@ public class RulesRegistrantIT { private final RulesKeyVerifier rulesKeyVerifier = new RulesKeyVerifier(); private final StartupRuleUpdater startupRuleUpdater = new StartupRuleUpdater(dbClient, system, uuidFactory, resolver); + private final NewRuleCreator newRuleCreator = new NewRuleCreator(dbClient, resolver, uuidFactory, system); @Before public void before() { @@ -187,9 +192,10 @@ public class RulesRegistrantIT { // verify db assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(3); RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1); - verifyRule(rule1); + verifyRule(rule1, RuleType.CODE_SMELL, BLOCKER); assertThat(rule1.isExternal()).isFalse(); assertThat(rule1.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.CONVENTIONAL); + assertThat(rule1.getDefaultImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity).containsOnly(tuple(SoftwareQuality.RELIABILITY, Severity.HIGH)); assertThat(rule1.getDefRemediationFunction()).isEqualTo(DebtRemediationFunction.Type.LINEAR_OFFSET.name()); assertThat(rule1.getDefRemediationGapMultiplier()).isEqualTo("5d"); assertThat(rule1.getDefRemediationBaseEffort()).isEqualTo("10h"); @@ -206,6 +212,7 @@ public class RulesRegistrantIT { // verify index RuleDto rule2 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY2); assertThat(rule2.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.EFFICIENT); + assertThat(rule2.getDefaultImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity).containsOnly(tuple(SoftwareQuality.MAINTAINABILITY, Severity.MEDIUM)); assertThat(ruleIndex.search(new RuleQuery(), new SearchOptions()).getUuids()).containsOnly(rule1.getUuid(), rule2.getUuid(), hotspotRule.getUuid()); verifyIndicesMarkedAsInitialized(); @@ -229,7 +236,7 @@ public class RulesRegistrantIT { // verify db assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(2); RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), EXTERNAL_RULE_KEY1); - verifyRule(rule1); + verifyRule(rule1, RuleType.CODE_SMELL, BLOCKER); assertThat(rule1.isExternal()).isTrue(); assertThat(rule1.getDefRemediationFunction()).isNull(); assertThat(rule1.getDefRemediationGapMultiplier()).isNull(); @@ -239,10 +246,10 @@ public class RulesRegistrantIT { verifyHotspot(hotspotRule); } - private void verifyRule(RuleDto rule) { + private void verifyRule(RuleDto rule, RuleType type, String expectedSeverity) { assertThat(rule.getName()).isEqualTo("One"); assertThat(rule.getDefaultRuleDescriptionSection().getContent()).isEqualTo("Description of One"); - assertThat(rule.getSeverityString()).isEqualTo(BLOCKER); + assertThat(rule.getSeverityString()).isEqualTo(expectedSeverity); assertThat(rule.getTags()).isEmpty(); assertThat(rule.getSystemTags()).containsOnly("tag1", "tag2", "tag3"); assertThat(rule.getConfigKey()).isEqualTo("config1"); @@ -250,7 +257,7 @@ public class RulesRegistrantIT { assertThat(rule.getCreatedAt()).isEqualTo(DATE1.getTime()); assertThat(rule.getScope()).isEqualTo(Scope.ALL); assertThat(rule.getUpdatedAt()).isEqualTo(DATE1.getTime()); - assertThat(rule.getType()).isEqualTo(RuleType.CODE_SMELL.getDbConstant()); + assertThat(rule.getType()).isEqualTo(type.getDbConstant()); assertThat(rule.getPluginKey()).isEqualTo(FAKE_PLUGIN_KEY); assertThat(rule.isAdHoc()).isFalse(); assertThat(rule.getEducationPrinciples()).containsOnly("concept1", "concept2", "concept3"); @@ -975,6 +982,7 @@ public class RulesRegistrantIT { .setRuleKey("rule1") .setRepositoryKey("findbugs") .setName("Rule One") + .setType(RuleType.CODE_SMELL) .setScope(Scope.ALL) .addRuleDescriptionSectionDto(createDefaultRuleDescriptionSection(uuidFactory.create(), "Rule one description")) .setDescriptionFormat(RuleDto.Format.HTML) @@ -1151,8 +1159,8 @@ public class RulesRegistrantIT { when(languages.get(any())).thenReturn(mock(Language.class)); reset(webServerRuleFinder); - RulesRegistrant task = new RulesRegistrant(loader, qProfileRules, dbClient, ruleIndexer, activeRuleIndexer, languages, system, webServerRuleFinder, uuidFactory, metadataIndex, - resolver, rulesKeyVerifier, startupRuleUpdater); + RulesRegistrant task = new RulesRegistrant(loader, qProfileRules, dbClient, ruleIndexer, activeRuleIndexer, languages, system, webServerRuleFinder, metadataIndex, + rulesKeyVerifier, startupRuleUpdater, newRuleCreator); task.start(); // Execute a commit to refresh session state as the task is using its own session db.getSession().commit(); @@ -1218,7 +1226,8 @@ public class RulesRegistrantIT { .setType(RuleType.CODE_SMELL) .setStatus(RuleStatus.BETA) .setGapDescription("java.S115.effortToFix") - .addEducationPrincipleKeys("concept1", "concept2", "concept3"); + .addEducationPrincipleKeys("concept1", "concept2", "concept3") + .addDefaultImpact(SoftwareQuality.RELIABILITY, Severity.HIGH); rule1.setDebtRemediationFunction(rule1.debtRemediationFunctions().linearWithOffset("5d", "10h")); rule1.createParam("param1").setDescription("parameter one").setDefaultValue("default1"); diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java index 9e6c5f13f8c..fc0ae871797 100644 --- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java +++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java @@ -39,6 +39,7 @@ import org.sonar.server.qualityprofile.builtin.BuiltInQualityProfilesUpdateListe import org.sonar.server.rule.AdvancedRuleDescriptionSectionsGenerator; import org.sonar.server.rule.LegacyHotspotRuleDescriptionSectionsGenerator; import org.sonar.server.rule.LegacyIssueRuleDescriptionSectionsGenerator; +import org.sonar.server.rule.registration.NewRuleCreator; import org.sonar.server.rule.registration.RulesKeyVerifier; import org.sonar.server.rule.registration.RulesRegistrant; import org.sonar.server.rule.RuleDescriptionSectionsGeneratorResolver; @@ -75,6 +76,7 @@ public class PlatformLevelStartup extends PlatformLevel { LegacyHotspotRuleDescriptionSectionsGenerator.class, LegacyIssueRuleDescriptionSectionsGenerator.class, RulesRegistrant.class, + NewRuleCreator.class, RulesKeyVerifier.class, StartupRuleUpdater.class, BuiltInQProfileLoader.class); -- 2.39.5