]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-22914 Support 2 built-in SCA rules
authorJulien HENRY <julien.henry@sonarsource.com>
Thu, 5 Sep 2024 15:22:09 +0000 (17:22 +0200)
committersonartech <sonartech@sonarsource.com>
Thu, 12 Sep 2024 20:02:54 +0000 (20:02 +0000)
server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java
server/sonar-webserver-core/src/test/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGeneratorTest.java
server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDefinitionsLoaderTest.java
server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorsTest.java
server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java

index 30914150e481d3811778b2d470a771bf3ccdbb90..3e3860db1362e85d3ad2299646e6eaacc3d245b2 100644 (file)
@@ -75,6 +75,7 @@ import org.sonar.server.es.EsTester;
 import org.sonar.server.es.SearchIdResult;
 import org.sonar.server.es.SearchOptions;
 import org.sonar.server.es.metadata.MetadataIndex;
+import org.sonar.server.plugins.DetectPluginChange;
 import org.sonar.server.plugins.ServerPluginRepository;
 import org.sonar.server.qualityprofile.ActiveRuleChange;
 import org.sonar.server.qualityprofile.QProfileRules;
@@ -95,6 +96,7 @@ import static java.util.Collections.emptySet;
 import static java.util.Collections.singletonList;
 import static org.apache.commons.lang3.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.tuple;
 import static org.mockito.ArgumentMatchers.any;
@@ -163,6 +165,7 @@ public class RulesRegistrantIT {
   private final StartupRuleUpdater startupRuleUpdater = new StartupRuleUpdater(dbClient, system, uuidFactory, resolver);
   private final NewRuleCreator newRuleCreator = new NewRuleCreator(resolver, uuidFactory, system);
   private final QualityProfileChangesUpdater qualityProfileChangesUpdater = mock();
+  private final DetectPluginChange detectPluginChange = mock();
 
   @Before
   public void before() {
@@ -190,7 +193,7 @@ public class RulesRegistrantIT {
 
   @Test
   public void insert_new_rules() {
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
 
     // verify db
     assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(3);
@@ -236,7 +239,7 @@ public class RulesRegistrantIT {
 
   @Test
   public void insert_new_external_rule() {
-    execute(new ExternalRuleRepository());
+    executeWithPluginRules(new ExternalRuleRepository());
 
     // verify db
     assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(2);
@@ -273,7 +276,7 @@ public class RulesRegistrantIT {
     String ruleKey = randomAlphanumeric(5);
 
     // register one rule
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule(ruleKey)
         .setName(randomAlphanumeric(5))
@@ -295,7 +298,7 @@ public class RulesRegistrantIT {
     verifyIndicesMarkedAsInitialized();
 
     // register no rule
-    execute(context -> context.createRepository("fake", "java").done());
+    executeWithPluginRules(context -> context.createRepository("fake", "java").done());
 
     // verify db
     assertThat(dbClient.ruleDao().selectAll(db.getSession()))
@@ -317,7 +320,7 @@ public class RulesRegistrantIT {
     int numberOfRules = 5000;
 
     // register many rules
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       IntStream.range(0, numberOfRules)
         .mapToObj(i -> "rule-" + i)
@@ -339,7 +342,7 @@ public class RulesRegistrantIT {
       .isNotEmpty();
 
     // register no rule
-    execute(context -> context.createRepository("fake", "java").done());
+    executeWithPluginRules(context -> context.createRepository("fake", "java").done());
 
     // verify db
     assertThat(dbClient.ruleDao().selectAll(db.getSession()))
@@ -360,14 +363,14 @@ public class RulesRegistrantIT {
     db.getDbClient().ruleRepositoryDao().insert(dbSession, singletonList(repository));
     dbSession.commit();
 
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
 
     assertThat(db.getDbClient().ruleRepositoryDao().selectAll(dbSession)).extracting(RuleRepositoryDto::getKey).containsOnly("fake");
   }
 
   @Test
   public void update_and_remove_rules_on_changes() {
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
     assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(3);
     RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1);
     RuleDto rule2 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY2);
@@ -383,7 +386,7 @@ public class RulesRegistrantIT {
     db.getSession().commit();
 
     system.setNow(DATE2.getTime());
-    execute(new FakeRepositoryV2());
+    executeWithPluginRules(new FakeRepositoryV2());
 
     verifyIndicesNotMarkedAsInitialized();
     // rule1 has been updated
@@ -413,7 +416,7 @@ public class RulesRegistrantIT {
     assertThat(dbClient.ruleRepositoryDao().selectAll(db.getSession())).extracting(RuleRepositoryDto::getKey).containsOnly("fake");
 
     system.setNow(DATE3.getTime());
-    execute(new FakeRepositoryV3());
+    executeWithPluginRules(new FakeRepositoryV3());
     rule3 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY3);
     assertThat(rule3.getDefaultRuleDescriptionSection().getContent()).isEqualTo("Rule Three V2");
     assertThat(rule3.getDescriptionFormat()).isEqualTo(RuleDto.Format.MARKDOWN);
@@ -440,7 +443,7 @@ public class RulesRegistrantIT {
 
   @Test
   public void add_new_tag() {
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule1")
         .setName("Rule One")
@@ -452,7 +455,7 @@ public class RulesRegistrantIT {
     RuleDto rule = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1);
     assertThat(rule.getSystemTags()).containsOnly("tag1");
 
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule1")
         .setName("Rule One")
@@ -467,7 +470,7 @@ public class RulesRegistrantIT {
 
   @Test
   public void add_new_security_standards() {
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule1")
         .setName("Rule One")
@@ -480,7 +483,7 @@ public class RulesRegistrantIT {
     RuleDto rule = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1);
     assertThat(rule.getSecurityStandards()).containsOnly("cwe:123", "owaspTop10-2021:a1");
 
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule1")
         .setName("Rule One")
@@ -497,7 +500,7 @@ public class RulesRegistrantIT {
   @Test
   public void update_only_rule_name() {
     system.setNow(DATE1.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule")
         .setName("Name1")
@@ -506,7 +509,7 @@ public class RulesRegistrantIT {
     });
 
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule")
         .setName("Name2")
@@ -526,7 +529,7 @@ public class RulesRegistrantIT {
   @Test
   public void update_template_rule_key_should_also_update_custom_rules() {
     system.setNow(DATE1.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("squid", "java");
       repo.createRule("rule")
         .setName("Name1")
@@ -547,7 +550,7 @@ public class RulesRegistrantIT {
     db.commit();
 
     // re-key rule
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("java", "java");
       repo.createRule("rule")
         .setName("Name1")
@@ -569,7 +572,7 @@ public class RulesRegistrantIT {
     String repository = "fake";
 
     system.setNow(DATE1.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repository, "java");
       repo.createRule(ruleKey1)
         .setName("Name1")
@@ -583,7 +586,7 @@ public class RulesRegistrantIT {
     assertThat(searchRule1.getTotal()).isOne();
 
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repository, "java");
       repo.createRule(ruleKey2)
         .setName("Name2")
@@ -611,7 +614,7 @@ public class RulesRegistrantIT {
     String repository2 = "fake2";
 
     system.setNow(DATE1.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repository1, "java");
       repo.createRule(ruleKey)
         .setName("Name1")
@@ -625,7 +628,7 @@ public class RulesRegistrantIT {
     assertThat(searchRule1.getTotal()).isOne();
 
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repository2, "java");
       repo.createRule(ruleKey)
         .setName("Name2")
@@ -652,7 +655,7 @@ public class RulesRegistrantIT {
     String name = "Name1";
     String description = "Description";
     system.setNow(DATE1.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repo1, "java");
       repo.createRule(ruleKey1)
         .setName(name)
@@ -665,7 +668,7 @@ public class RulesRegistrantIT {
       .containsOnly(rule1.getUuid());
 
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repo2, "java");
       repo.createRule(ruleKey2)
         .setName(name)
@@ -701,7 +704,7 @@ public class RulesRegistrantIT {
     String repository2 = "fake2";
 
     system.setNow(DATE1.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repository1, "java");
       repo.createRule(ruleKey1)
         .setName("Name1")
@@ -714,7 +717,7 @@ public class RulesRegistrantIT {
       .containsOnly(rule1.getUuid());
 
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository(repository2, "java");
       repo.createRule(ruleKey2)
         .setName("Name2")
@@ -736,7 +739,7 @@ public class RulesRegistrantIT {
   @Test
   public void update_only_rule_description() {
     system.setNow(DATE1.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule")
         .setName("Name")
@@ -745,7 +748,7 @@ public class RulesRegistrantIT {
     });
 
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule")
         .setName("Name")
@@ -772,7 +775,7 @@ public class RulesRegistrantIT {
     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 -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule")
         .setName("Name")
@@ -791,7 +794,7 @@ public class RulesRegistrantIT {
     RuleDescriptionSection section4updatedWithContext1 = createRuleDescriptionSection(ROOT_CAUSE_SECTION_KEY, section4noContext.getHtmlContent(), "ctx_1");
     RuleDescriptionSection section4updatedWithContext2 = createRuleDescriptionSection(ROOT_CAUSE_SECTION_KEY, section4noContext.getHtmlContent(), "ctx_2");
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       repo.createRule("rule")
         .setName("Name")
@@ -850,7 +853,7 @@ public class RulesRegistrantIT {
   public void rule_previously_created_as_adhoc_becomes_none_adhoc() {
     RuleDto rule = db.rules().insert(r -> r.setRepositoryKey("external_fake").setIsExternal(true).setIsAdHoc(true));
     system.setNow(DATE2.getTime());
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createExternalRepository("fake", rule.getLanguage());
       repo.createRule(rule.getRuleKey())
         .setName(rule.getName())
@@ -869,7 +872,7 @@ public class RulesRegistrantIT {
       .setIsExternal(true)
       .setIsAdHoc(false));
 
-    execute();
+    executeWithPluginRules();
 
     RuleDto reloaded = dbClient.ruleDao().selectByKey(db.getSession(), rule.getKey()).get();
     assertThat(reloaded.getStatus()).isEqualTo(REMOVED);
@@ -882,7 +885,7 @@ public class RulesRegistrantIT {
       .setIsExternal(true)
       .setIsAdHoc(true));
 
-    execute();
+    executeWithPluginRules();
 
     RuleDto reloaded = dbClient.ruleDao().selectByKey(db.getSession(), rule.getKey()).get();
     assertThat(reloaded.getStatus()).isEqualTo(READY);
@@ -892,11 +895,11 @@ public class RulesRegistrantIT {
   public void disable_then_enable_rule() {
     // Install rule
     system.setNow(DATE1.getTime());
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
 
     // Uninstall rule
     system.setNow(DATE2.getTime());
-    execute();
+    executeWithPluginRules();
 
     RuleDto rule = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1);
     assertThat(rule.getStatus()).isEqualTo(REMOVED);
@@ -904,7 +907,7 @@ public class RulesRegistrantIT {
 
     // Re-install rule
     system.setNow(DATE3.getTime());
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
 
     rule = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1);
     assertThat(rule.getStatus()).isEqualTo(RuleStatus.BETA);
@@ -913,11 +916,11 @@ public class RulesRegistrantIT {
 
   @Test
   public void do_not_update_rules_when_no_changes() {
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
     assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(3);
 
     system.setNow(DATE2.getTime());
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
 
     RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1);
     assertThat(rule1.getCreatedAt()).isEqualTo(DATE1.getTime());
@@ -926,7 +929,7 @@ public class RulesRegistrantIT {
 
   @Test
   public void do_not_update_already_removed_rules() {
-    execute(new FakeRepositoryV1());
+    executeWithPluginRules(new FakeRepositoryV1());
     assertThat(dbClient.ruleDao().selectAll(db.getSession())).hasSize(3);
 
     RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RULE_KEY1);
@@ -937,7 +940,7 @@ public class RulesRegistrantIT {
     assertThat(rule2.getStatus()).isEqualTo(READY);
 
     system.setNow(DATE2.getTime());
-    execute(new FakeRepositoryV2());
+    executeWithPluginRules(new FakeRepositoryV2());
 
     // On MySQL, need to update a rule otherwise rule2 will be seen as READY, but why ???
     dbClient.ruleDao().update(db.getSession(), rule1);
@@ -951,7 +954,7 @@ public class RulesRegistrantIT {
     assertThat(ruleIndex.search(new RuleQuery(), new SearchOptions()).getUuids()).containsOnly(rule1.getUuid(), rule3.getUuid());
 
     system.setNow(DATE3.getTime());
-    execute(new FakeRepositoryV2());
+    executeWithPluginRules(new FakeRepositoryV2());
     db.getSession().commit();
 
     // -> rule2 is still removed, but not update at DATE3
@@ -964,7 +967,7 @@ public class RulesRegistrantIT {
 
   @Test
   public void mass_insert() {
-    execute(new BigRepository());
+    executeWithPluginRules(new BigRepository());
     assertThat(db.countRowsOfTable("rules")).isEqualTo(BigRepository.SIZE);
     assertThat(db.countRowsOfTable("rules_parameters")).isEqualTo(BigRepository.SIZE * 20);
     assertThat(es.getIds(RuleIndexDefinition.TYPE_RULE)).hasSize(BigRepository.SIZE);
@@ -972,7 +975,7 @@ public class RulesRegistrantIT {
 
   @Test
   public void manage_repository_extensions() {
-    execute(new FindbugsRepository(), new FbContribRepository());
+    executeWithPluginRules(new FindbugsRepository(), new FbContribRepository());
     List<RuleDto> rules = dbClient.ruleDao().selectAll(db.getSession());
     assertThat(rules).hasSize(2);
     for (RuleDto rule : rules) {
@@ -995,7 +998,7 @@ public class RulesRegistrantIT {
     db.getSession().commit();
 
     // Synchronize rule without tag
-    execute(new FindbugsRepository());
+    executeWithPluginRules(new FindbugsRepository());
 
     List<RuleDto> rules = dbClient.ruleDao().selectAll(db.getSession());
     assertThat(rules).hasSize(1).extracting(RuleDto::getKey, RuleDto::getSystemTags)
@@ -1004,13 +1007,13 @@ public class RulesRegistrantIT {
 
   @Test
   public void rules_that_deprecate_previous_rule_must_be_recorded() {
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       createRule(repo, "rule1");
       repo.done();
     });
 
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       createRule(repo, "newKey")
         .addDeprecatedRuleKey("fake", "rule1")
@@ -1026,13 +1029,13 @@ public class RulesRegistrantIT {
 
   @Test
   public void rules_that_remove_deprecated_key_must_remove_records() {
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       createRule(repo, "rule1");
       repo.done();
     });
 
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       createRule(repo, "newKey")
         .addDeprecatedRuleKey("fake", "rule1")
@@ -1044,7 +1047,7 @@ public class RulesRegistrantIT {
     Set<DeprecatedRuleKeyDto> deprecatedRuleKeys = dbClient.ruleDao().selectAllDeprecatedRuleKeys(db.getSession());
     assertThat(deprecatedRuleKeys).hasSize(2);
 
-    execute(context -> {
+    executeWithPluginRules(context -> {
       NewRepository repo = context.createRepository("fake", "java");
       createRule(repo, "newKey");
       repo.done();
@@ -1058,7 +1061,7 @@ public class RulesRegistrantIT {
   @Test
   public void declaring_two_rules_with_same_deprecated_RuleKey_should_throw_ISE() {
     assertThatThrownBy(() -> {
-      execute(context -> {
+      executeWithPluginRules(context -> {
         NewRepository repo = context.createRepository("fake", "java");
         createRule(repo, "newKey1")
           .addDeprecatedRuleKey("fake", "old");
@@ -1074,7 +1077,7 @@ public class RulesRegistrantIT {
   @Test
   public void declaring_a_rule_with_a_deprecated_RuleKey_still_used_should_throw_ISE() {
     assertThatThrownBy(() -> {
-      execute(context -> {
+      executeWithPluginRules(context -> {
         NewRepository repo = context.createRepository("fake", "java");
         createRule(repo, "newKey1");
         createRule(repo, "newKey2")
@@ -1089,12 +1092,12 @@ public class RulesRegistrantIT {
   @Test
   public void updating_the_deprecated_to_a_new_ruleKey_should_throw_an_ISE() {
     // On this new rule add a deprecated key
-    execute(context -> createRule(context, "javascript", "javascript", "s103",
+    executeWithPluginRules(context -> createRule(context, "javascript", "javascript", "s103",
       r -> r.addDeprecatedRuleKey("javascript", "linelength")));
 
     assertThatThrownBy(() -> {
       // This rule should have been moved to another repository
-      execute(context -> createRule(context, "javascript", "sonarjs", "s103",
+      executeWithPluginRules(context -> createRule(context, "javascript", "sonarjs", "s103",
         r -> r.addDeprecatedRuleKey("javascript", "linelength")));
     })
       .isInstanceOf(IllegalStateException.class)
@@ -1104,12 +1107,12 @@ public class RulesRegistrantIT {
 
   @Test
   public void deprecate_rule_that_deprecated_another_rule() {
-    execute(context -> createRule(context, "javascript", "javascript", "s103"));
-    execute(context -> createRule(context, "javascript", "javascript", "s104",
+    executeWithPluginRules(context -> createRule(context, "javascript", "javascript", "s103"));
+    executeWithPluginRules(context -> createRule(context, "javascript", "javascript", "s104",
       r -> r.addDeprecatedRuleKey("javascript", "s103")));
 
     // This rule should have been moved to another repository
-    execute(context -> createRule(context, "javascript", "sonarjs", "s105",
+    executeWithPluginRules(context -> createRule(context, "javascript", "sonarjs", "s105",
       r -> r.addDeprecatedRuleKey("javascript", "s103")
         .addDeprecatedRuleKey("javascript", "s104")));
   }
@@ -1117,7 +1120,7 @@ public class RulesRegistrantIT {
   @Test
   public void declaring_a_rule_with_an_existing_RuleKey_still_used_should_throw_IAE() {
     assertThatThrownBy(() -> {
-      execute(context -> {
+      executeWithPluginRules(context -> {
         NewRepository repo = context.createRepository("fake", "java");
         createRule(repo, "newKey1");
         createRule(repo, "newKey1");
@@ -1137,7 +1140,7 @@ public class RulesRegistrantIT {
     ActiveRuleChange arChange = new ActiveRuleChange(DEACTIVATED, ActiveRuleDto.createFor(qProfileDto, ruleDto), ruleDto);
     when(qProfileRules.deleteRule(any(DbSession.class), eq(ruleDto))).thenReturn(List.of(arChange));
     //WHEN
-    execute(context -> context.createRepository("fake", "java").done());
+    executeWithPluginRules(context -> context.createRepository("fake", "java").done());
     //THEN
     List<QProfileChangeDto> qProfileChangeDtos = dbClient.qProfileChangeDao().selectByQuery(db.getSession(), new QProfileChangeQuery(qProfileDto.getKee()));
     assertThat(qProfileChangeDtos).extracting(QProfileChangeDto::getRulesProfileUuid, QProfileChangeDto::getChangeType, QProfileChangeDto::getSqVersion)
@@ -1150,22 +1153,53 @@ public class RulesRegistrantIT {
     RuleDto removedRuleDto = db.rules().insert(RuleKey.of("old_repo", "removed_rule"));
     RuleDto renamedRuleDto = db.rules().insert(RuleKey.of("old_repo", "renamed_rule"));
     //WHEN
-    execute(context -> createRule(context, "java", "new_repo", renamedRuleDto.getRuleKey(),
+    executeWithPluginRules(context -> createRule(context, "java", "new_repo", renamedRuleDto.getRuleKey(),
       rule -> rule.addDeprecatedRuleKey(renamedRuleDto.getRepositoryKey(), renamedRuleDto.getRuleKey())));
     //THEN
     verify(qProfileRules).deleteRule(any(DbSession.class), eq(removedRuleDto));
   }
 
-  private void execute(RulesDefinition... defs) {
+  @Test
+  public void builtin_rules_should_be_updated_even_if_no_plugin_updates() {
+    RulesDefinition builtInRepoV1 = context -> createRule(context, "builtin", "sca", "rule1", rule -> rule.setName("Name before"));
+    RulesDefinition pluginRepo = context -> createRule(context, "java", "java", "rule2");
+
+    ServerPluginRepository pluginRepository = mock(ServerPluginRepository.class);
+    when(pluginRepository.getPluginKey(builtInRepoV1)).thenReturn(null);
+    when(pluginRepository.getPluginKey(pluginRepo)).thenReturn(FAKE_PLUGIN_KEY);
+    RuleDefinitionsLoader loader1 = new RuleDefinitionsLoader(pluginRepository, new RulesDefinition[] {builtInRepoV1, pluginRepo});
+    when(detectPluginChange.anyPluginChanged()).thenReturn(true);
+    execute(loader1);
+
+    RuleDto builtInRulev1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RuleKey.of("sca", "rule1"));
+    assertThat(builtInRulev1.getName()).isEqualTo("Name before");
+
+    RulesDefinition builtInRepoV2 = context -> createRule(context, "builtin", "sca", "rule1", rule -> rule.setName("Name after"));
+    when(pluginRepository.getPluginKey(builtInRepoV2)).thenReturn(null);
+    RuleDefinitionsLoader loader2 = new RuleDefinitionsLoader(pluginRepository, new RulesDefinition[] {builtInRepoV2, pluginRepo});
+    when(detectPluginChange.anyPluginChanged()).thenReturn(false);
+    execute(loader2);
+
+    RuleDto builtInRulev2 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RuleKey.of("sca", "rule1"));
+    assertThat(builtInRulev2.getName()).isEqualTo("Name after");
+
+    assertThatCode(() -> dbClient.ruleDao().selectOrFailByKey(db.getSession(), RuleKey.of("java", "rule2"))).doesNotThrowAnyException();
+  }
+
+
+  private void executeWithPluginRules(RulesDefinition... defs) {
     ServerPluginRepository pluginRepository = mock(ServerPluginRepository.class);
     when(pluginRepository.getPluginKey(any(RulesDefinition.class))).thenReturn(FAKE_PLUGIN_KEY);
     RuleDefinitionsLoader loader = new RuleDefinitionsLoader(pluginRepository, defs);
-    Languages languages = mock(Languages.class);
-    when(languages.get(any())).thenReturn(mock(Language.class));
+    when(detectPluginChange.anyPluginChanged()).thenReturn(true);
+    execute(loader);
+  }
+
+  private void execute(RuleDefinitionsLoader loader) {
     reset(webServerRuleFinder);
 
-    RulesRegistrant task = new RulesRegistrant(loader, qProfileRules, dbClient, ruleIndexer, activeRuleIndexer, languages, system, webServerRuleFinder, metadataIndex,
-      rulesKeyVerifier, startupRuleUpdater, newRuleCreator, qualityProfileChangesUpdater, sonarQubeVersion);
+    RulesRegistrant task = new RulesRegistrant(loader, qProfileRules, dbClient, ruleIndexer, activeRuleIndexer, system, webServerRuleFinder, metadataIndex,
+      rulesKeyVerifier, startupRuleUpdater, newRuleCreator, qualityProfileChangesUpdater, sonarQubeVersion, detectPluginChange);
     task.start();
     // Execute a commit to refresh session state as the task is using its own session
     db.getSession().commit();
index 42c23791290388a330d3ee0b01d10af752909106..40c60e4523e8a1f758f70ec7347006858e33a2fe 100644 (file)
@@ -22,7 +22,6 @@ package org.sonar.server.rule;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
-import org.elasticsearch.common.util.set.Sets;
 import org.jetbrains.annotations.Nullable;
 import org.sonar.api.server.rule.Context;
 import org.sonar.api.server.rule.RuleDescriptionSection;
@@ -31,42 +30,23 @@ import org.sonar.core.util.UuidFactory;
 import org.sonar.db.rule.RuleDescriptionSectionContextDto;
 import org.sonar.db.rule.RuleDescriptionSectionDto;
 
-import static org.sonar.api.rules.RuleType.*;
-
 public class AdvancedRuleDescriptionSectionsGenerator implements RuleDescriptionSectionsGenerator {
   private final UuidFactory uuidFactory;
-  private final LegacyIssueRuleDescriptionSectionsGenerator legacyIssueRuleDescriptionSectionsGenerator;
 
-  public AdvancedRuleDescriptionSectionsGenerator(UuidFactory uuidFactory, LegacyIssueRuleDescriptionSectionsGenerator legacyIssueRuleDescriptionSectionsGenerator) {
+  public AdvancedRuleDescriptionSectionsGenerator(UuidFactory uuidFactory) {
     this.uuidFactory = uuidFactory;
-    this.legacyIssueRuleDescriptionSectionsGenerator = legacyIssueRuleDescriptionSectionsGenerator;
   }
 
   @Override
   public boolean isGeneratorForRule(RulesDefinition.Rule rule) {
-    return !rule.ruleDescriptionSections().isEmpty() && skipHotspotRulesForSonar16635(rule);
-  }
-
-  private static boolean skipHotspotRulesForSonar16635(RulesDefinition.Rule rule) {
-    return !SECURITY_HOTSPOT.equals(rule.type());
+    return !rule.ruleDescriptionSections().isEmpty();
   }
 
   @Override
   public Set<RuleDescriptionSectionDto> generateSections(RulesDefinition.Rule rule) {
-    Set<RuleDescriptionSectionDto> advancedSections = rule.ruleDescriptionSections().stream()
+    return rule.ruleDescriptionSections().stream()
       .map(this::toRuleDescriptionSectionDto)
       .collect(Collectors.toSet());
-    return addLegacySectionToAdvancedSections(advancedSections, rule);
-  }
-
-  /**
-   * This was done to preserve backward compatibility with SonarLint until they stop using htmlDesc field in api/rules/[show|search] endpoints, see SONAR-16635
-   * @deprecated the method should be removed once SonarLint supports rules.descriptionSections fields, I.E in 10.x
-   */
-  @Deprecated(since = "9.6", forRemoval = true)
-  private Set<RuleDescriptionSectionDto> addLegacySectionToAdvancedSections(Set<RuleDescriptionSectionDto> advancedSections, RulesDefinition.Rule rule) {
-    Set<RuleDescriptionSectionDto> legacySection = legacyIssueRuleDescriptionSectionsGenerator.generateSections(rule);
-    return Sets.union(advancedSections, legacySection);
   }
 
   private RuleDescriptionSectionDto toRuleDescriptionSectionDto(RuleDescriptionSection section) {
index 4d1ca46ed98b2f54ea56b2f4933d77ea72229c6e..5c381578b014cbac4fc92f94439f3a211e41af25 100644 (file)
@@ -46,9 +46,7 @@ public class LegacyHotspotRuleDescriptionSectionsGenerator implements RuleDescri
 
   @Override
   public boolean isGeneratorForRule(RulesDefinition.Rule rule) {
-    // To prevent compatibility issues with SonarLint, this Generator is used for all hotspots rules, regardless of if they expose advanced sections or not. See SONAR-16635.
-    // In the future, the generator should not be used for advanced rules (add condition && rule.ruleDescriptionSections().isEmpty())
-    return SECURITY_HOTSPOT.equals(rule.type());
+    return SECURITY_HOTSPOT.equals(rule.type()) && rule.ruleDescriptionSections().isEmpty();
   }
 
   @Override
@@ -115,7 +113,6 @@ public class LegacyHotspotRuleDescriptionSectionsGenerator implements RuleDescri
       .collect(Collectors.toSet());
   }
 
-
   private static String[] extractSection(String beginning, String description) {
     String endSection = "<h2>";
     int beginningIndex = description.indexOf(beginning);
index 55fc93dcc70a35fe6b9712f936c4dd421aead9b8..67c6d2ecedb94c9307ba6411d8065ed0cf4d7bfb 100644 (file)
  */
 package org.sonar.server.rule;
 
-import org.sonar.api.server.rule.RulesDefinition;
+import java.util.Objects;
+import java.util.function.Predicate;
 import org.sonar.api.impl.server.RulesDefinitionContext;
+import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.server.plugins.ServerPluginRepository;
 import org.springframework.beans.factory.annotation.Autowired;
 
@@ -30,13 +32,13 @@ import org.springframework.beans.factory.annotation.Autowired;
  */
 public class RuleDefinitionsLoader {
 
-  private final RulesDefinition[] pluginDefs;
+  private final RulesDefinition[] rulesDefinitions;
   private final ServerPluginRepository serverPluginRepository;
 
   @Autowired(required = false)
-  public RuleDefinitionsLoader(ServerPluginRepository serverPluginRepository, RulesDefinition[] pluginDefs) {
+  public RuleDefinitionsLoader(ServerPluginRepository serverPluginRepository, RulesDefinition[] rulesDefinitions) {
     this.serverPluginRepository = serverPluginRepository;
-    this.pluginDefs = pluginDefs;
+    this.rulesDefinitions = rulesDefinitions;
   }
 
   /**
@@ -47,11 +49,22 @@ public class RuleDefinitionsLoader {
     this(serverPluginRepository, new RulesDefinition[0]);
   }
 
-  public RulesDefinition.Context load() {
+  public RulesDefinition.Context loadFromPlugins() {
+    return load(Predicate.not(Objects::isNull));
+  }
+
+  public RulesDefinition.Context loadBuiltIn() {
+    return load(Objects::isNull);
+  }
+
+  private RulesDefinition.Context load(Predicate<String> pluginKeyPredicate) {
     RulesDefinition.Context context = new RulesDefinitionContext();
-    for (RulesDefinition pluginDefinition : pluginDefs) {
-      context.setCurrentPluginKey(serverPluginRepository.getPluginKey(pluginDefinition));
-      pluginDefinition.define(context);
+    for (RulesDefinition rulesDefinition : rulesDefinitions) {
+      var pluginKey = serverPluginRepository.getPluginKey(rulesDefinition);
+      if (pluginKeyPredicate.test(pluginKey)) {
+        context.setCurrentPluginKey(pluginKey);
+        rulesDefinition.define(context);
+      }
     }
     context.setCurrentPluginKey(null);
     return context;
index eaba17b5b3adb5b348c464136dc98bc8564a7e89..75328fccf26bc7c2fb25ee86e253c14d0c3ae555 100644 (file)
@@ -33,7 +33,7 @@ public class RuleDescriptionSectionsGeneratorResolver {
     this.ruleDescriptionSectionsGenerators = ruleDescriptionSectionsGenerators;
   }
 
-  public RuleDescriptionSectionsGenerator getRuleDescriptionSectionsGenerator(RulesDefinition.Rule ruleDef) {
+  RuleDescriptionSectionsGenerator getRuleDescriptionSectionsGenerator(RulesDefinition.Rule ruleDef) {
     Set<RuleDescriptionSectionsGenerator> generatorsFound = ruleDescriptionSectionsGenerators.stream()
       .filter(generator -> generator.isGeneratorForRule(ruleDef))
       .collect(toSet());
index 5197b44a79227460f3c3f964dfc59b530e513348..9a022988bab48497239689fd49d7722da051fb94 100644 (file)
@@ -32,7 +32,6 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.sonar.api.Startable;
-import org.sonar.api.resources.Languages;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.server.rule.RulesDefinition;
@@ -49,6 +48,7 @@ import org.sonar.db.rule.RuleDescriptionSectionDto;
 import org.sonar.db.rule.RuleDto;
 import org.sonar.db.rule.RuleRepositoryDto;
 import org.sonar.server.es.metadata.MetadataIndex;
+import org.sonar.server.plugins.DetectPluginChange;
 import org.sonar.server.qualityprofile.ActiveRuleChange;
 import org.sonar.server.qualityprofile.QProfileRules;
 import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
@@ -74,7 +74,6 @@ public class RulesRegistrant implements Startable {
   private final DbClient dbClient;
   private final RuleIndexer ruleIndexer;
   private final ActiveRuleIndexer activeRuleIndexer;
-  private final Languages languages;
   private final System2 system2;
   private final WebServerRuleFinder webServerRuleFinder;
   private final MetadataIndex metadataIndex;
@@ -83,17 +82,17 @@ public class RulesRegistrant implements Startable {
   private final NewRuleCreator newRuleCreator;
   private final QualityProfileChangesUpdater qualityProfileChangesUpdater;
   private final SonarQubeVersion sonarQubeVersion;
+  private final DetectPluginChange detectPluginChange;
 
   public RulesRegistrant(RuleDefinitionsLoader defLoader, QProfileRules qProfileRules, DbClient dbClient, RuleIndexer ruleIndexer,
-    ActiveRuleIndexer activeRuleIndexer, Languages languages, System2 system2, WebServerRuleFinder webServerRuleFinder,
+    ActiveRuleIndexer activeRuleIndexer, System2 system2, WebServerRuleFinder webServerRuleFinder,
     MetadataIndex metadataIndex, RulesKeyVerifier rulesKeyVerifier, StartupRuleUpdater startupRuleUpdater,
-    NewRuleCreator newRuleCreator, QualityProfileChangesUpdater qualityProfileChangesUpdater, SonarQubeVersion sonarQubeVersion) {
+    NewRuleCreator newRuleCreator, QualityProfileChangesUpdater qualityProfileChangesUpdater, SonarQubeVersion sonarQubeVersion, DetectPluginChange detectPluginChange) {
     this.defLoader = defLoader;
     this.qProfileRules = qProfileRules;
     this.dbClient = dbClient;
     this.ruleIndexer = ruleIndexer;
     this.activeRuleIndexer = activeRuleIndexer;
-    this.languages = languages;
     this.system2 = system2;
     this.webServerRuleFinder = webServerRuleFinder;
     this.metadataIndex = metadataIndex;
@@ -102,28 +101,40 @@ public class RulesRegistrant implements Startable {
     this.newRuleCreator = newRuleCreator;
     this.qualityProfileChangesUpdater = qualityProfileChangesUpdater;
     this.sonarQubeVersion = sonarQubeVersion;
+    this.detectPluginChange = detectPluginChange;
   }
 
   @Override
   public void start() {
     Profiler profiler = Profiler.create(LOG).startInfo("Register rules");
     try (DbSession dbSession = dbClient.openSession(true)) {
-      List<RulesDefinition.Repository> repositories = defLoader.load().repositories();
-      RulesRegistrationContext rulesRegistrationContext = RulesRegistrationContext.create(dbClient, dbSession);
-      rulesKeyVerifier.verifyRuleKeyConsistency(repositories, rulesRegistrationContext);
+      var anyPluginChanged = detectPluginChange.anyPluginChanged();
+      RulesRegistrationContext rulesRegistrationContext = RulesRegistrationContext.create(dbClient, dbSession, !anyPluginChanged);
 
-      for (RulesDefinition.ExtendedRepository repoDef : repositories) {
-        if (languages.get(repoDef.language()) != null) {
-          Set<PluginRuleUpdate> pluginRuleUpdates = registerRules(rulesRegistrationContext, repoDef.rules(), dbSession);
+      List<RulesDefinition.Repository> rulesRepositories = new ArrayList<>(defLoader.loadBuiltIn().repositories());
+      if (anyPluginChanged) {
+        LOG.info("Some plugins have changed, triggering loading of rules from plugins");
+        rulesRepositories.addAll(defLoader.loadFromPlugins().repositories());
+      }
+      rulesKeyVerifier.verifyRuleKeyConsistency(rulesRepositories, rulesRegistrationContext);
+
+      persistRepositories(dbSession, rulesRepositories, anyPluginChanged);
+
+      for (RulesDefinition.Repository repoDef : rulesRepositories) {
+        if (repoDef.language() == null) {
+          throw new IllegalStateException("Language is mandatory for repository " + repoDef.key());
+        }
+        Set<PluginRuleUpdate> pluginRuleUpdates = registerRules(rulesRegistrationContext, repoDef.rules(), dbSession);
+        if (!repoDef.isExternal()) {
+          // External rules are not part of quality profiles
           qualityProfileChangesUpdater.createQprofileChangesForRuleUpdates(dbSession, pluginRuleUpdates);
-          dbSession.commit();
         }
+        dbSession.commit();
       }
       processRemainingDbRules(rulesRegistrationContext, dbSession);
-      List<ActiveRuleChange> changes = removeActiveRulesOnStillExistingRepositories(dbSession, rulesRegistrationContext, repositories);
+      List<ActiveRuleChange> changes = anyPluginChanged ? removeActiveRulesOnStillExistingRepositories(dbSession, rulesRegistrationContext, rulesRepositories) : List.of();
       dbSession.commit();
 
-      persistRepositories(dbSession, repositories);
       // FIXME lack of resiliency, active rules index is corrupted if rule index fails
       // to be updated. Only a single DB commit should be executed.
       ruleIndexer.commitAndIndex(dbSession, rulesRegistrationContext.getAllModified().map(RuleDto::getUuid).collect(Collectors.toSet()));
@@ -147,7 +158,7 @@ public class RulesRegistrant implements Startable {
     }
   }
 
-  private void persistRepositories(DbSession dbSession, List<RulesDefinition.Repository> repositories) {
+  private void persistRepositories(DbSession dbSession, List<RulesDefinition.Repository> repositories, boolean deleteMissing) {
     List<String> keys = repositories.stream().map(RulesDefinition.Repository::key).toList();
     Set<String> existingKeys = dbClient.ruleRepositoryDao().selectAllKeys(dbSession);
 
@@ -157,7 +168,9 @@ public class RulesRegistrant implements Startable {
 
     dbClient.ruleRepositoryDao().update(dbSession, dtos.getOrDefault(true, emptyList()));
     dbClient.ruleRepositoryDao().insert(dbSession, dtos.getOrDefault(false, emptyList()));
-    dbClient.ruleRepositoryDao().deleteIfKeyNotIn(dbSession, keys);
+    if (deleteMissing) {
+      dbClient.ruleRepositoryDao().deleteIfKeyNotIn(dbSession, keys);
+    }
     dbSession.commit();
   }
 
@@ -344,10 +357,10 @@ public class RulesRegistrant implements Startable {
    * Remove active rules on repositories that still exists.
    * <p/>
    * For instance, if the javascript repository do not provide anymore some rules, active rules related to this rules will be removed.
-   * But if the javascript repository do not exists anymore, then related active rules will not be removed.
+   * But if the javascript repository does not exist anymore, then related active rules will not be removed.
    * <p/>
    * The side effect of this approach is that extended repositories will not be managed the same way.
-   * If an extended repository do not exists anymore, then related active rules will be removed.
+   * If an extended repository does not exist anymore, then related active rules will be removed.
    */
   private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession dbSession, RulesRegistrationContext recorder, List<RulesDefinition.Repository> context) {
     Set<String> existingAndRenamedRepositories = getExistingAndRenamedRepositories(recorder, context);
index e9c6a8c2d0605f45df1afd2d7590c6792681ba00..7c1496773a64b8575c5917251517235662138126 100644 (file)
@@ -191,8 +191,9 @@ class RulesRegistrationContext {
     checkState(known.contains(ruleDto), "unknown RuleDto");
   }
 
-  static RulesRegistrationContext create(DbClient dbClient, DbSession dbSession) {
+  static RulesRegistrationContext create(DbClient dbClient, DbSession dbSession, boolean onlyBuiltIn) {
     Map<RuleKey, RuleDto> allRules = dbClient.ruleDao().selectAll(dbSession).stream()
+      .filter(rule -> !onlyBuiltIn || rule.getPluginKey() == null)
       .collect(Collectors.toMap(RuleDto::getKey, Function.identity()));
     Map<String, Set<SingleDeprecatedRuleKey>> existingDeprecatedKeysById = loadDeprecatedRuleKeys(dbClient, dbSession);
     Map<String, List<RuleParamDto>> ruleParamsByRuleUuid = loadAllRuleParameters(dbClient, dbSession);
index d651c2da65484d4f731d2bfaea655997a9ac14f9..7948270684e5b070de2daf9a9606eafe0e46b2af 100644 (file)
@@ -91,72 +91,56 @@ public class AdvancedRuleDescriptionSectionsGeneratorTest {
   @Mock
   private RulesDefinition.Rule rule;
 
-  @Mock
-  private RuleDescriptionSectionDto LEGACY_SECTION;
-
-  @Mock
-  private LegacyIssueRuleDescriptionSectionsGenerator legacyIssueRuleDescriptionSectionsGenerator;
-
   @InjectMocks
   private AdvancedRuleDescriptionSectionsGenerator generator;
 
   @Before
   public void before() {
     when(uuidFactory.create()).thenReturn(UUID_1).thenReturn(UUID_2);
-    when(legacyIssueRuleDescriptionSectionsGenerator.generateSections(rule)).thenReturn(Set.of(LEGACY_SECTION));
   }
 
   @Test
-  public void generateSections_whenOneSection_createsOneSectionAndDefault() {
+  public void generateSections_whenOneSection_createsOneSection() {
     when(rule.ruleDescriptionSections()).thenReturn(List.of(SECTION_1));
 
     Set<RuleDescriptionSectionDto> ruleDescriptionSectionDtos = generator.generateSections(rule);
 
     assertThat(ruleDescriptionSectionDtos)
       .usingRecursiveFieldByFieldElementComparator()
-      .containsExactlyInAnyOrder(EXPECTED_SECTION_1, LEGACY_SECTION);
+      .containsExactlyInAnyOrder(EXPECTED_SECTION_1);
   }
 
   @Test
-  public void generateSections_whenTwoSections_createsTwoSectionsAndDefault() {
+  public void generateSections_whenTwoSections_createsTwoSections() {
     when(rule.ruleDescriptionSections()).thenReturn(List.of(SECTION_1, SECTION_2));
 
     Set<RuleDescriptionSectionDto> ruleDescriptionSectionDtos = generator.generateSections(rule);
 
     assertThat(ruleDescriptionSectionDtos)
       .usingRecursiveFieldByFieldElementComparator()
-      .containsExactlyInAnyOrder(EXPECTED_SECTION_1, EXPECTED_SECTION_2, LEGACY_SECTION);
+      .containsExactlyInAnyOrder(EXPECTED_SECTION_1, EXPECTED_SECTION_2);
   }
 
   @Test
-  public void generateSections_whenTwoContextSpecificSections_createsTwoSectionsAndDefault() {
+  public void generateSections_whenTwoContextSpecificSections_createsTwoSections() {
     when(rule.ruleDescriptionSections()).thenReturn(List.of(SECTION_3_WITH_CTX_1, SECTION_3_WITH_CTX_2));
 
     Set<RuleDescriptionSectionDto> ruleDescriptionSectionDtos = generator.generateSections(rule);
 
     assertThat(ruleDescriptionSectionDtos)
       .usingRecursiveFieldByFieldElementComparator()
-      .containsExactlyInAnyOrder(EXPECTED_SECTION_3_WITH_CTX_1, EXPECTED_SECTION_3_WITH_CTX_2, LEGACY_SECTION);
+      .containsExactlyInAnyOrder(EXPECTED_SECTION_3_WITH_CTX_1, EXPECTED_SECTION_3_WITH_CTX_2);
   }
 
   @Test
-  public void generateSections_whenContextSpecificSectionsAndNonContextSpecificSection_createsTwoSectionsAndDefault() {
+  public void generateSections_whenContextSpecificSectionsAndNonContextSpecificSection_createsTwoSections() {
     when(rule.ruleDescriptionSections()).thenReturn(List.of(SECTION_1, SECTION_3_WITH_CTX_2));
 
     Set<RuleDescriptionSectionDto> ruleDescriptionSectionDtos = generator.generateSections(rule);
 
     assertThat(ruleDescriptionSectionDtos)
       .usingRecursiveFieldByFieldElementComparator()
-      .containsExactlyInAnyOrder(EXPECTED_SECTION_1, EXPECTED_SECTION_3_WITH_CTX_2, LEGACY_SECTION);
-  }
-
-  @Test
-  public void generateSections_whenNoSections_returnsDefault() {
-    when(rule.ruleDescriptionSections()).thenReturn(emptyList());
-
-    Set<RuleDescriptionSectionDto> ruleDescriptionSectionDtos = generator.generateSections(rule);
-
-    assertThat(ruleDescriptionSectionDtos).containsOnly(LEGACY_SECTION);
+      .containsExactlyInAnyOrder(EXPECTED_SECTION_1, EXPECTED_SECTION_3_WITH_CTX_2);
   }
 
 }
index 70fa43036405db1d307eae3c9a0c62d48cd507e2..28b566f9eddfecfd22fddb3444487cc0fabe4062 100644 (file)
@@ -25,25 +25,46 @@ import org.sonar.server.plugins.ServerPluginRepository;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class RuleDefinitionsLoaderTest {
 
   @Test
   public void no_definitions() {
-    RulesDefinition.Context context = new RuleDefinitionsLoader(mock(ServerPluginRepository.class)).load();
+    RulesDefinition.Context context = new RuleDefinitionsLoader(mock(ServerPluginRepository.class)).loadFromPlugins();
 
     assertThat(context.repositories()).isEmpty();
   }
 
   @Test
-  public void load_definitions() {
-    RulesDefinition.Context context = new RuleDefinitionsLoader(mock(ServerPluginRepository.class),
+  public void load_FromPlugins_definitions_only_returns_from_plugins() {
+    var serverPluginRepository = mock(ServerPluginRepository.class);
+    var findbugsDefinitions = new FindbugsDefinitions();
+    var builtInJavaDefinitions = new JavaDefinitions();
+    when(serverPluginRepository.getPluginKey(findbugsDefinitions)).thenReturn("findbugs");
+    when(serverPluginRepository.getPluginKey(builtInJavaDefinitions)).thenReturn(null);
+    RulesDefinition.Context context = new RuleDefinitionsLoader(serverPluginRepository,
       new RulesDefinition[] {
-        new FindbugsDefinitions(), new JavaDefinitions()
-      }).load();
+        findbugsDefinitions, builtInJavaDefinitions
+      }).loadFromPlugins();
 
-    assertThat(context.repositories()).hasSize(2);
+    assertThat(context.repositories()).hasSize(1);
     assertThat(context.repository("findbugs")).isNotNull();
+  }
+
+  @Test
+  public void load_builtin_definitions_only_returns_builtin() {
+    var serverPluginRepository = mock(ServerPluginRepository.class);
+    var findbugsDefinitions = new FindbugsDefinitions();
+    var builtInJavaDefinitions = new JavaDefinitions();
+    when(serverPluginRepository.getPluginKey(findbugsDefinitions)).thenReturn("findbugs");
+    when(serverPluginRepository.getPluginKey(builtInJavaDefinitions)).thenReturn(null);
+    RulesDefinition.Context context = new RuleDefinitionsLoader(serverPluginRepository,
+      new RulesDefinition[] {
+        findbugsDefinitions, builtInJavaDefinitions
+      }).loadBuiltIn();
+
+    assertThat(context.repositories()).hasSize(1);
     assertThat(context.repository("java")).isNotNull();
   }
 
index c787de8d930bda9c3428984f8bcbfb259bd0e820..98940c061781dace781cc97f696cd24658088198 100644 (file)
@@ -73,29 +73,30 @@ public class RuleDescriptionSectionsGeneratorsTest {
   @Parameterized.Parameters(name = "{index} = {0}")
   public static List<RuleDescriptionGeneratorTestData> testData() {
     return Arrays.asList(
-      // ISSUES
+      // ISSUES WITHOUT SECTIONS
       aRuleOfType(BUG).html(null).md(null).expectedGenerator(LEGACY_ISSUE).build(),
       aRuleOfType(BUG).html(HTML_CONTENT).md(null).expectedGenerator(LEGACY_ISSUE).addExpectedSection(DEFAULT_HTML_SECTION_1).build(),
       aRuleOfType(BUG).html(null).md(MD_CONTENT).expectedGenerator(LEGACY_ISSUE).addExpectedSection(DEFAULT_MD_SECTION_1).build(),
       aRuleOfType(BUG).html(HTML_CONTENT).md(MD_CONTENT).expectedGenerator(LEGACY_ISSUE).addExpectedSection(DEFAULT_HTML_SECTION_1).build(),
       aRuleOfType(CODE_SMELL).html(HTML_CONTENT).md(MD_CONTENT).expectedGenerator(LEGACY_ISSUE).addExpectedSection(DEFAULT_HTML_SECTION_1).build(),
       aRuleOfType(VULNERABILITY).html(HTML_CONTENT).md(MD_CONTENT).expectedGenerator(LEGACY_ISSUE).addExpectedSection(DEFAULT_HTML_SECTION_1).build(),
-      // HOTSPOT
+      // HOTSPOT WITHOUT SECTIONS
       aRuleOfType(SECURITY_HOTSPOT).html(null).md(null).expectedGenerator(LEGACY_HOTSPOT).build(),
       aRuleOfType(SECURITY_HOTSPOT).html(HTML_CONTENT).md(null).expectedGenerator(LEGACY_HOTSPOT).addExpectedSection(DEFAULT_HTML_HOTSPOT_SECTION_1).addExpectedSection(LEGACY_HTML_SECTION).build(),
       aRuleOfType(SECURITY_HOTSPOT).html(null).md(MD_CONTENT).expectedGenerator(LEGACY_HOTSPOT).addExpectedSection(DEFAULT_MD_HOTSPOT_SECTION_1).addExpectedSection(LEGACY_MD_SECTION).build(),
       aRuleOfType(SECURITY_HOTSPOT).html(HTML_CONTENT).md(MD_CONTENT).expectedGenerator(LEGACY_HOTSPOT).addExpectedSection(DEFAULT_HTML_HOTSPOT_SECTION_1).addExpectedSection(LEGACY_HTML_SECTION).build(),
-      // ADVANCED RULES
+      // RULES WITH SECTIONS
       aRuleOfType(BUG).html(null).md(null).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
-      aRuleOfType(BUG).html(HTML_CONTENT).md(null).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).addExpectedSection(LEGACY_HTML_SECTION).build(),
-      aRuleOfType(BUG).html(null).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).addExpectedSection(LEGACY_MD_SECTION).build(),
-      aRuleOfType(BUG).html(HTML_CONTENT).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).addExpectedSection(LEGACY_HTML_SECTION).build(),
+      aRuleOfType(BUG).html(HTML_CONTENT).md(null).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
+      aRuleOfType(BUG).html(null).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
+      aRuleOfType(BUG).html(HTML_CONTENT).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
       aRuleOfType(BUG).html(HTML_CONTENT).md(MD_CONTENT).addSection(SECTION_1).addSection(SECTION_2).expectedGenerator(ADVANCED_RULE)
-        .addExpectedSection(HTML_SECTION_1).addExpectedSection(HTML_SECTION_2).addExpectedSection(LEGACY_HTML_SECTION).build(),
-      aRuleOfType(SECURITY_HOTSPOT).html(null).md(null).addSection(SECTION_1).expectedGenerator(LEGACY_HOTSPOT).build(),
-      aRuleOfType(SECURITY_HOTSPOT).html(HTML_CONTENT).md(null).addSection(SECTION_1).expectedGenerator(LEGACY_HOTSPOT).addExpectedSection(DEFAULT_HTML_HOTSPOT_SECTION_1).addExpectedSection(LEGACY_HTML_SECTION).build(),
-      aRuleOfType(SECURITY_HOTSPOT).html(null).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(LEGACY_HOTSPOT).addExpectedSection(DEFAULT_MD_HOTSPOT_SECTION_1).addExpectedSection(LEGACY_MD_SECTION).build(),
-      aRuleOfType(SECURITY_HOTSPOT).html(HTML_CONTENT).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(LEGACY_HOTSPOT).addExpectedSection(DEFAULT_HTML_HOTSPOT_SECTION_1).addExpectedSection(LEGACY_HTML_SECTION).build()
+        .addExpectedSection(HTML_SECTION_1).addExpectedSection(HTML_SECTION_2).build(),
+      aRuleOfType(SECURITY_HOTSPOT).html(null).md(null).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
+      aRuleOfType(SECURITY_HOTSPOT).html(HTML_CONTENT).md(null).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
+      aRuleOfType(SECURITY_HOTSPOT).html(null).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
+      aRuleOfType(SECURITY_HOTSPOT).html(HTML_CONTENT).md(MD_CONTENT).addSection(SECTION_1).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).build(),
+      aRuleOfType(SECURITY_HOTSPOT).html(HTML_CONTENT).md(MD_CONTENT).addSection(SECTION_1).addSection(SECTION_2).expectedGenerator(ADVANCED_RULE).addExpectedSection(HTML_SECTION_1).addExpectedSection(HTML_SECTION_2).build()
     );
   }
 
@@ -106,7 +107,7 @@ public class RuleDescriptionSectionsGeneratorsTest {
 
   private final RuleDescriptionSectionsGenerator legacyHotspotRuleDescriptionSectionsGenerator = new LegacyHotspotRuleDescriptionSectionsGenerator(uuidFactory);
   private final LegacyIssueRuleDescriptionSectionsGenerator legacyIssueRuleDescriptionSectionsGenerator = new LegacyIssueRuleDescriptionSectionsGenerator(uuidFactory);
-  private final RuleDescriptionSectionsGenerator advancedRuleDescriptionSectionsGenerator = new AdvancedRuleDescriptionSectionsGenerator(uuidFactory, legacyIssueRuleDescriptionSectionsGenerator);
+  private final RuleDescriptionSectionsGenerator advancedRuleDescriptionSectionsGenerator = new AdvancedRuleDescriptionSectionsGenerator(uuidFactory);
 
   Map<RuleDescriptionSectionGeneratorIdentifier, RuleDescriptionSectionsGenerator> idToGenerator = ImmutableMap.<RuleDescriptionSectionGeneratorIdentifier, RuleDescriptionSectionsGenerator>builder()
     .put(ADVANCED_RULE, advancedRuleDescriptionSectionsGenerator)
index eb016d45ba21c0cff4ed6c19e16527b103c2a7e5..c7272a49d908b066b5734ad5e0e4206af3c5a568 100644 (file)
@@ -67,18 +67,19 @@ public class PlatformLevelStartup extends PlatformLevel {
 
     addIfStartupLeader(
       IndexerStartupTask.class);
-    addIfStartupLeaderAndPluginsChanged(
-      RegisterMetrics.class,
-      RegisterQualityGates.class,
+    addIfStartupLeader(
       RuleDescriptionSectionsGeneratorResolver.class,
       AdvancedRuleDescriptionSectionsGenerator.class,
       LegacyHotspotRuleDescriptionSectionsGenerator.class,
       LegacyIssueRuleDescriptionSectionsGenerator.class,
       RulesRegistrant.class,
-      QualityProfileChangesUpdater.class,
       NewRuleCreator.class,
       RulesKeyVerifier.class,
       StartupRuleUpdater.class,
+      QualityProfileChangesUpdater.class);
+    addIfStartupLeaderAndPluginsChanged(
+      RegisterMetrics.class,
+      RegisterQualityGates.class,
       BuiltInQProfileLoader.class);
     addIfStartupLeader(
       BuiltInQualityProfilesUpdateListener.class,