From: Duarte Meneses Date: Wed, 26 Apr 2023 15:59:41 +0000 (-0500) Subject: Revert "SONAR-19050 Populate characteristic RuleDto during RegisterRule" X-Git-Tag: 10.1.0.73491~382 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=590b2685f9b05b8d2f68d03a0d2c885de54889d6;p=sonarqube.git Revert "SONAR-19050 Populate characteristic RuleDto during RegisterRule" This reverts commit e92b736e1b4eb842d26409aacc4789bf96ca2cd3. --- diff --git a/build.gradle b/build.gradle index d227c981b78..2bbf56068c0 100644 --- a/build.gradle +++ b/build.gradle @@ -215,8 +215,8 @@ subprojects { dependency 'org.sonarsource.kotlin:sonar-kotlin-plugin:2.13.0.2116' dependency 'org.sonarsource.slang:sonar-ruby-plugin:1.12.0.4259' dependency 'org.sonarsource.slang:sonar-scala-plugin:1.12.0.4259' - dependency "org.sonarsource.api.plugin:sonar-plugin-api:$pluginApiVersion" - dependency "org.sonarsource.api.plugin:sonar-plugin-api-test-fixtures:$pluginApiVersion" + dependency "org.sonarsource.api.plugin:sonar-plugin-api:9.16.0.500" + dependency "org.sonarsource.api.plugin:sonar-plugin-api-test-fixtures:9.16.0.500" dependency 'org.sonarsource.xml:sonar-xml-plugin:2.7.0.3820' dependency 'org.sonarsource.iac:sonar-iac-plugin:1.15.0.3752' dependency 'org.sonarsource.text:sonar-text-plugin:2.0.2.1090' diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/rule/RuleDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/rule/RuleDaoIT.java index ec138783b03..59f9bd5f29d 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/rule/RuleDaoIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/rule/RuleDaoIT.java @@ -521,7 +521,6 @@ public class RuleDaoIT { .setSecurityStandards(newHashSet("owaspTop10:a1", "cwe:123")) .setScope(Scope.ALL) .setType(RuleType.BUG) - .setCharacteristic(RuleCharacteristic.PORTABLE) .setUpdatedAt(2_000_000_000_000L); underTest.update(db.getSession(), ruleToUpdate); @@ -547,7 +546,6 @@ public class RuleDaoIT { assertThat(ruleDto.getSecurityStandards()).containsOnly("owaspTop10:a1", "cwe:123"); assertThat(ruleDto.getScope()).isEqualTo(Scope.ALL); assertThat(ruleDto.getType()).isEqualTo(RuleType.BUG.getDbConstant()); - assertThat(ruleDto.getCharacteristic()).isEqualTo(RuleCharacteristic.PORTABLE); assertThat(ruleDto.getCreatedAt()).isEqualTo(rule.getCreatedAt()); assertThat(ruleDto.getUpdatedAt()).isEqualTo(2_000_000_000_000L); assertThat(ruleDto.getDescriptionFormat()).isEqualTo(RuleDto.Format.MARKDOWN); 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 5a01ffd4ff4..dbacc89df55 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 @@ -406,7 +406,7 @@ public class RuleDto { return characteristic != null ? characteristic : RuleTypeToRuleCharacteristicConverter.convertToRuleCharacteristic(type); } - public RuleDto setCharacteristic(@Nullable RuleCharacteristic characteristic) { + public RuleDto setCharacteristic(RuleCharacteristic characteristic) { this.characteristic = characteristic; return this; } diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml index 8bf29f59950..e481e9a3930 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml @@ -344,7 +344,6 @@ security_standards=#{securityStandardsField,jdbcType=VARCHAR}, scope=#{scope,jdbcType=VARCHAR}, rule_type=#{type,jdbcType=TINYINT}, - characteristic=#{characteristic,jdbcType=VARCHAR}, note_data=#{noteData,jdbcType=CLOB}, note_user_uuid=#{noteUserUuid,jdbcType=VARCHAR}, note_created_at=#{noteCreatedAt,jdbcType=BIGINT}, 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 fc701c189ae..f4b09aaa120 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 @@ -41,6 +41,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.sonar.db.rule.RuleDto.ERROR_MESSAGE_SECTION_ALREADY_EXISTS; import static org.sonar.db.rule.RuleTesting.newRule; +@RunWith(DataProviderRunner.class) public class RuleDtoTest { public static final String SECTION_KEY = "section key"; diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java index 6cf042a4273..6dfc0e417fa 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java @@ -43,7 +43,6 @@ import org.sonar.api.resources.Languages; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleScope; import org.sonar.api.rule.RuleStatus; -import org.sonar.api.rules.RuleCharacteristic; import org.sonar.api.rules.RuleType; import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.api.server.rule.RulesDefinition; @@ -101,6 +100,7 @@ public class RegisterRules implements Startable { private final MetadataIndex metadataIndex; private final RuleDescriptionSectionsGeneratorResolver ruleDescriptionSectionsGeneratorResolver; + public RegisterRules(RuleDefinitionsLoader defLoader, QProfileRules qProfileRules, DbClient dbClient, RuleIndexer ruleIndexer, ActiveRuleIndexer activeRuleIndexer, Languages languages, System2 system2, WebServerRuleFinder webServerRuleFinder, UuidFactory uuidFactory, MetadataIndex metadataIndex, @@ -209,7 +209,7 @@ public class RegisterRules implements Startable { RuleDto rule = dbRulesByRuleUuid.get(ruleUuid); if (rule == null) { LOG.warn("Could not retrieve rule with uuid %s referenced by a deprecated rule key. " + - "The following deprecated rule keys seem to be referencing a non-existing rule", + "The following deprecated rule keys seem to be referencing a non-existing rule", ruleUuid, entry.getValue()); } else { entry.getValue().forEach(d -> rulesByKey.put(d.getOldRuleKeyAsRuleKey(), rule)); @@ -268,10 +268,10 @@ public class RegisterRules implements Startable { private Stream getAllModified() { return Stream.of( - created.stream(), - updated.stream(), - removed.stream(), - renamed.keySet().stream()) + created.stream(), + updated.stream(), + removed.stream(), + renamed.keySet().stream()) .flatMap(s -> s); } @@ -391,8 +391,7 @@ public class RegisterRules implements Startable { .setGapDescription(ruleDef.gapDescription()) .setSystemTags(ruleDef.tags()) .setSecurityStandards(ruleDef.securityStandards()) - .setType(ruleDef.type()) - .setCharacteristic(ruleDef.characteristic()) + .setType(RuleType.valueOf(ruleDef.type().name())) .setScope(toDtoScope(ruleDef.scope())) .setIsExternal(ruleDef.repository().isExternal()) .setIsAdHoc(false) @@ -487,17 +486,11 @@ public class RegisterRules implements Startable { dto.setLanguage(def.repository().language()); changed = true; } - RuleType type = def.type(); + RuleType type = RuleType.valueOf(def.type().name()); if (!Objects.equals(dto.getType(), type.getDbConstant())) { dto.setType(type); changed = true; } - RuleCharacteristic characteristic = def.characteristic(); - if (!Objects.equals(dto.getCharacteristic(), characteristic)) { - dto.setCharacteristic(characteristic); - changed = true; - } - if (dto.isAdHoc()) { dto.setIsAdHoc(false); changed = true; @@ -670,9 +663,9 @@ public class RegisterRules implements Startable { changed = true; } else if (dto.getSystemTags().size() != ruleDef.tags().size() || !dto.getSystemTags().containsAll(ruleDef.tags())) { - dto.setSystemTags(ruleDef.tags()); - changed = true; - } + dto.setSystemTags(ruleDef.tags()); + changed = true; + } return changed; } @@ -684,9 +677,9 @@ public class RegisterRules implements Startable { changed = true; } else if (dto.getSecurityStandards().size() != ruleDef.securityStandards().size() || !dto.getSecurityStandards().containsAll(ruleDef.securityStandards())) { - dto.setSecurityStandards(ruleDef.securityStandards()); - changed = true; - } + dto.setSecurityStandards(ruleDef.securityStandards()); + changed = true; + } return changed; } @@ -818,8 +811,8 @@ public class RegisterRules implements Startable { private static Set getExistingAndRenamedRepositories(RegisterRulesContext 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(toSet()); } diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java index ff98036da12..89a8a76de27 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java @@ -42,7 +42,6 @@ import org.sonar.api.resources.Languages; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleScope; import org.sonar.api.rule.RuleStatus; -import org.sonar.api.rules.RuleCharacteristic; import org.sonar.api.rules.RuleType; import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.api.server.rule.Context; @@ -165,7 +164,8 @@ public class RegisterRulesTest { .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())); }); @@ -210,7 +210,6 @@ public class RegisterRulesTest { assertThat(hotspotRule.getCreatedAt()).isEqualTo(RegisterRulesTest.DATE1.getTime()); assertThat(hotspotRule.getUpdatedAt()).isEqualTo(RegisterRulesTest.DATE1.getTime()); assertThat(hotspotRule.getType()).isEqualTo(RuleType.SECURITY_HOTSPOT.getDbConstant()); - assertThat(hotspotRule.getCharacteristic()).isEqualTo(RuleCharacteristic.SECURE); assertThat(hotspotRule.getSecurityStandards()).containsExactly("cwe:1", "cwe:123", "cwe:863", "owaspTop10-2021:a1", "owaspTop10-2021:a3"); } @@ -221,7 +220,7 @@ public class RegisterRulesTest { // verify db assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(2); RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), EXTERNAL_RULE_KEY1); - verifyExternalRule(rule1); + verifyRule(rule1); assertThat(rule1.isExternal()).isTrue(); assertThat(rule1.getDefRemediationFunction()).isNull(); assertThat(rule1.getDefRemediationGapMultiplier()).isNull(); @@ -232,15 +231,6 @@ public class RegisterRulesTest { } private void verifyRule(RuleDto rule) { - verifyCommonsFields(rule); - } - - private void verifyExternalRule(RuleDto rule) { - verifyCommonsFields(rule); - assertThat(rule.getCharacteristic()).isEqualTo(RuleCharacteristic.COMPLIANT); - } - - private void verifyCommonsFields(RuleDto rule) { assertThat(rule.getName()).isEqualTo("One"); assertThat(rule.getDefaultRuleDescriptionSection().getContent()).isEqualTo("Description of One"); assertThat(rule.getSeverityString()).isEqualTo(BLOCKER); @@ -422,10 +412,9 @@ public class RegisterRulesTest { assertThat(rule1.getNoteUserUuid()).isEqualTo("marius"); assertThat(rule1.getStatus()).isEqualTo(READY); assertThat(rule1.getType()).isEqualTo(RuleType.BUG.getDbConstant()); - assertThat(rule1.getCharacteristic()).isEqualTo(RuleCharacteristic.PORTABLE); 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 @@ -676,7 +665,7 @@ public class RegisterRulesTest { @DataProvider public static Object[][] allRenamingCases() { - return new Object[][] { + return new Object[][]{ {"repo1", "rule1", "repo1", "rule2"}, {"repo1", "rule1", "repo2", "rule1"}, {"repo1", "rule1", "repo2", "rule2"}, @@ -759,7 +748,7 @@ public class RegisterRulesTest { 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); execute(context -> { @@ -1119,15 +1108,15 @@ public class RegisterRulesTest { @Test public 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 execute(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) .contains(tuple(qProfileDto.getRulesProfileUuid(), "DEACTIVATED")); @@ -1135,13 +1124,13 @@ public class RegisterRulesTest { @Test public 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 execute(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)); } @@ -1170,7 +1159,6 @@ public class RegisterRulesTest { .setInternalKey("config1") .setTags("tag1", "tag2", "tag3") .setType(RuleType.CODE_SMELL) - .setCharacteristic(RuleCharacteristic.ROBUST) .setStatus(RuleStatus.BETA); } @@ -1182,7 +1170,6 @@ public class RegisterRulesTest { .setHtmlDescription("Description of One") .setSeverity(BLOCKER) .setType(RuleType.CODE_SMELL) - .setCharacteristic(RuleCharacteristic.TESTED) .setStatus(RuleStatus.BETA); Arrays.stream(consumers).forEach(c -> c.accept(newRule)); @@ -1220,7 +1207,6 @@ public class RegisterRulesTest { .setTags("tag1", "tag2", "tag3") .setScope(RuleScope.ALL) .setType(RuleType.CODE_SMELL) - .setCharacteristic(RuleCharacteristic.CLEAR) .setStatus(RuleStatus.BETA) .setGapDescription("java.S115.effortToFix") .addEducationPrincipleKeys("concept1", "concept2", "concept3"); @@ -1233,7 +1219,6 @@ public class RegisterRulesTest { .setName("Hotspot") .setHtmlDescription("Minimal hotspot") .setType(RuleType.SECURITY_HOTSPOT) - .setCharacteristic(RuleCharacteristic.SECURE) .addOwaspTop10(Y2021, OwaspTop10.A1, OwaspTop10.A3) .addCwe(1, 123, 863); @@ -1261,7 +1246,6 @@ public class RegisterRulesTest { // tag2 and tag3 removed, tag4 added .setTags("tag1", "tag4") .setType(RuleType.BUG) - .setCharacteristic(RuleCharacteristic.PORTABLE) .setStatus(READY) .setGapDescription("java.S115.effortToFix.v2") .addEducationPrincipleKeys("concept1", "concept4"); @@ -1303,7 +1287,6 @@ public class RegisterRulesTest { .setTags("tag1", "tag2", "tag3") .setScope(RuleScope.ALL) .setType(RuleType.CODE_SMELL) - .setCharacteristic(RuleCharacteristic.COMPLIANT) .setStatus(RuleStatus.BETA) .addEducationPrincipleKeys("concept1", "concept2", "concept3"); @@ -1311,7 +1294,6 @@ public class RegisterRulesTest { .setName("Hotspot") .setHtmlDescription("Minimal hotspot") .setType(RuleType.SECURITY_HOTSPOT) - .setCharacteristic(RuleCharacteristic.SECURE) .addOwaspTop10(Y2021, OwaspTop10.A1, OwaspTop10.A3) .addCwe(1, 123, 863);