]> source.dussan.org Git - sonarqube.git/commitdiff
Revert "SONAR-19050 Populate characteristic RuleDto during RegisterRule"
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Wed, 26 Apr 2023 15:59:41 +0000 (10:59 -0500)
committersonartech <sonartech@sonarsource.com>
Fri, 28 Apr 2023 20:02:58 +0000 (20:02 +0000)
This reverts commit e92b736e1b4eb842d26409aacc4789bf96ca2cd3.

build.gradle
server/sonar-db-dao/src/it/java/org/sonar/db/rule/RuleDaoIT.java
server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java
server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml
server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDtoTest.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java
server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java

index d227c981b78ed5dbdb2ab9fd60afb4df6a80a956..2bbf56068c036d0d6245040fe9c613194db91ec8 100644 (file)
@@ -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'
index ec138783b0399e8865f14f550f73b7d3151c140d..59f9bd5f29d52ec1c6acad1d6cb7f9bc7da22e1b 100644 (file)
@@ -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);
index 5a01ffd4ff4ef3491ef47ff679fcfe6ea8289b33..dbacc89df559862c416fb6f89a462eb6762ebaaa 100644 (file)
@@ -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;
   }
index 8bf29f59950b3e3ab101eecf078c57fe28c61a12..e481e9a3930b7fb504c44ca9988811a7a5816902 100644 (file)
       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},
index fc701c189ae54fbb20f698417dad4f698e9396e5..f4b09aaa1202d2f3acd6aeea23d696383c7479eb 100644 (file)
@@ -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";
index 6cf042a4273ab08876451338c3e6e88fd0fcc0a2..6dfc0e417fa611029621f5132cb6d70418ef1ff5 100644 (file)
@@ -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<RuleDto> 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<String> getExistingAndRenamedRepositories(RegisterRulesContext recorder, Collection<RulesDefinition.Repository> 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());
   }
 
index ff98036da127f830cee5a696a0357705b12f5b6b..89a8a76de27b08bb98bc9275250a1ecd58113cfe 100644 (file)
@@ -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<QProfileChangeDto> 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);