From f405b2169cf6598535504bbc3b8c5d40f81763e1 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Thu, 5 Sep 2024 17:22:09 +0200 Subject: [PATCH] SONAR-22914 Support 2 built-in SCA rules --- .../rule/registration/RulesRegistrantIT.java | 162 +++++++++++------- ...ancedRuleDescriptionSectionsGenerator.java | 26 +-- ...tspotRuleDescriptionSectionsGenerator.java | 5 +- .../server/rule/RuleDefinitionsLoader.java | 29 +++- ...eDescriptionSectionsGeneratorResolver.java | 2 +- .../rule/registration/RulesRegistrant.java | 49 ++++-- .../RulesRegistrationContext.java | 3 +- ...dRuleDescriptionSectionsGeneratorTest.java | 32 +--- .../rule/RuleDefinitionsLoaderTest.java | 33 +++- ...RuleDescriptionSectionsGeneratorsTest.java | 25 +-- .../platformlevel/PlatformLevelStartup.java | 9 +- 11 files changed, 210 insertions(+), 165 deletions(-) diff --git a/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java b/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java index 30914150e48..3e3860db136 100644 --- a/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java +++ b/server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java @@ -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 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 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 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 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(); diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java index 42c23791290..40c60e4523e 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java @@ -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 generateSections(RulesDefinition.Rule rule) { - Set 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 addLegacySectionToAdvancedSections(Set advancedSections, RulesDefinition.Rule rule) { - Set legacySection = legacyIssueRuleDescriptionSectionsGenerator.generateSections(rule); - return Sets.union(advancedSections, legacySection); } private RuleDescriptionSectionDto toRuleDescriptionSectionDto(RuleDescriptionSection section) { diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java index 4d1ca46ed98..5c381578b01 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java @@ -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 = "

"; int beginningIndex = description.indexOf(beginning); diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java index 55fc93dcc70..67c6d2ecedb 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java @@ -19,8 +19,10 @@ */ 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 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; diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java index eaba17b5b3a..75328fccf26 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java @@ -33,7 +33,7 @@ public class RuleDescriptionSectionsGeneratorResolver { this.ruleDescriptionSectionsGenerators = ruleDescriptionSectionsGenerators; } - public RuleDescriptionSectionsGenerator getRuleDescriptionSectionsGenerator(RulesDefinition.Rule ruleDef) { + RuleDescriptionSectionsGenerator getRuleDescriptionSectionsGenerator(RulesDefinition.Rule ruleDef) { Set generatorsFound = ruleDescriptionSectionsGenerators.stream() .filter(generator -> generator.isGeneratorForRule(ruleDef)) .collect(toSet()); 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 5197b44a792..9a022988bab 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 @@ -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 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 pluginRuleUpdates = registerRules(rulesRegistrationContext, repoDef.rules(), dbSession); + List 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 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 changes = removeActiveRulesOnStillExistingRepositories(dbSession, rulesRegistrationContext, repositories); + List 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 repositories) { + private void persistRepositories(DbSession dbSession, List repositories, boolean deleteMissing) { List keys = repositories.stream().map(RulesDefinition.Repository::key).toList(); Set 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. *

* 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. *

* 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 removeActiveRulesOnStillExistingRepositories(DbSession dbSession, RulesRegistrationContext recorder, List context) { Set existingAndRenamedRepositories = getExistingAndRenamedRepositories(recorder, context); diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java index e9c6a8c2d06..7c1496773a6 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java @@ -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 allRules = dbClient.ruleDao().selectAll(dbSession).stream() + .filter(rule -> !onlyBuiltIn || rule.getPluginKey() == null) .collect(Collectors.toMap(RuleDto::getKey, Function.identity())); Map> existingDeprecatedKeysById = loadDeprecatedRuleKeys(dbClient, dbSession); Map> ruleParamsByRuleUuid = loadAllRuleParameters(dbClient, dbSession); diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGeneratorTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGeneratorTest.java index d651c2da654..7948270684e 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGeneratorTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGeneratorTest.java @@ -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 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 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 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 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 ruleDescriptionSectionDtos = generator.generateSections(rule); - - assertThat(ruleDescriptionSectionDtos).containsOnly(LEGACY_SECTION); + .containsExactlyInAnyOrder(EXPECTED_SECTION_1, EXPECTED_SECTION_3_WITH_CTX_2); } } diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDefinitionsLoaderTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDefinitionsLoaderTest.java index 70fa4303640..28b566f9edd 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDefinitionsLoaderTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDefinitionsLoaderTest.java @@ -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(); } diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorsTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorsTest.java index c787de8d930..98940c06178 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorsTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorsTest.java @@ -73,29 +73,30 @@ public class RuleDescriptionSectionsGeneratorsTest { @Parameterized.Parameters(name = "{index} = {0}") public static List 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 idToGenerator = ImmutableMap.builder() .put(ADVANCED_RULE, advancedRuleDescriptionSectionsGenerator) 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 eb016d45ba2..c7272a49d90 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 @@ -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, -- 2.39.5